Hi,
Here's an attempt to improve ADC access in the pcf50633 driver.
I've introduced a method for synchronously reading from the ADC.
The adc external interface consists only of adc_async_read and
adc_sync_read.
Also, don't the queue head/tail manipulation functions require locking ?
Please review.
pcf50633.c | 154 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 107 insertions(+), 47 deletions(-)
diff --git a/drivers/i2c/chips/pcf50633.c b/drivers/i2c/chips/pcf50633.c
index 13b3158..68bc8c0 100644
--- a/drivers/i2c/chips/pcf50633.c
+++ b/drivers/i2c/chips/pcf50633.c
@@ -117,6 +117,21 @@ enum pcf50633_suspend_states {
PCF50633_SS_COMPLETED_RESUME,
};
+#define ADC_REQUEST_TYPE_SYNC 1
+#define ADC_REQUEST_TYPE_ASYNC 2
+
+struct adc_request {
+ int mux;
+ int avg;
+ int request_type;
+
+ /* For async reads */
+ void (*callback)(struct pcf50633_data *pcf, int result);
+
+ /* for sync reads */
+ struct completion completion;
+ int result;
+};
struct pcf50633_data {
struct i2c_client *client;
@@ -160,14 +175,10 @@ struct pcf50633_data {
int coldplug_done; /* cleared by probe, set by first work service */
int flag_bat_voltage_read; /* ipc to /sys batt voltage read func */
- int charger_adc_result_raw;
- enum charger_type charger_type;
-
/* we have a FIFO of ADC measurement requests that are used only by
* the workqueue service code after the ADC completion interrupt
*/
- int adc_queue_mux[MAX_ADC_FIFO_DEPTH]; /* which ADC input to use */
- int adc_queue_avg[MAX_ADC_FIFO_DEPTH]; /* amount of averaging */
+ struct adc_request *adc_queue[MAX_ADC_FIFO_DEPTH]; /* amount of
averaging */
int adc_queue_head; /* head owned by foreground code */
int adc_queue_tail; /* tail owned by service code */
@@ -299,7 +310,7 @@ static void async_adc_read_setup(struct pcf50633_data *pcf,
}
-static u_int16_t async_adc_complete(struct pcf50633_data *pcf)
+static u_int16_t adc_read_result(struct pcf50633_data *pcf)
{
u_int16_t ret = (__reg_read(pcf, PCF50633_REG_ADCS1) << 2) |
(__reg_read(pcf, PCF50633_REG_ADCS3) &
@@ -556,8 +567,12 @@ static int interpret_charger_type_from_adc(struct
pcf50633_data *pcf,
static void configure_pmu_for_charger(struct pcf50633_data *pcf,
- enum charger_type type)
+ int adc_result_raw)
{
+ int type;
+
+ type = interpret_charger_type_from_adc(
+ pcf, adc_result_raw);
switch (type) {
case CHARGER_TYPE_NONE:
pcf50633_usb_curlim_set(pcf, 0);
@@ -605,16 +620,16 @@ static void trigger_next_adc_job_if_any(struct
pcf50633_data *pcf)
if (pcf->adc_queue_head == pcf->adc_queue_tail)
return;
async_adc_read_setup(pcf,
- pcf->adc_queue_mux[pcf->adc_queue_tail],
- pcf->adc_queue_avg[pcf->adc_queue_tail]);
+ pcf->adc_queue[pcf->adc_queue_tail]->mux,
+ pcf->adc_queue[pcf->adc_queue_tail]->avg);
}
-static void add_request_to_adc_queue(struct pcf50633_data *pcf,
- int mux, int avg)
+
+static void
+adc_add_request_to_queue(struct pcf50633_data *pcf, struct adc_request *req)
{
int old_head = pcf->adc_queue_head;
- pcf->adc_queue_mux[pcf->adc_queue_head] = mux;
- pcf->adc_queue_avg[pcf->adc_queue_head] = avg;
+ pcf->adc_queue[pcf->adc_queue_head] = req;
pcf->adc_queue_head = (pcf->adc_queue_head + 1) &
(MAX_ADC_FIFO_DEPTH - 1);
@@ -624,6 +639,49 @@ static void add_request_to_adc_queue(struct pcf50633_data
*pcf,
trigger_next_adc_job_if_any(pcf);
}
+static int adc_sync_read(struct pcf50633_data *pcf, int mux, int avg)
+{
+
+ struct adc_request *req;
+ int result;
+
+ req = kmalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ req->mux = mux;
+ req->avg = avg;
+ req->request_type = ADC_REQUEST_TYPE_SYNC;
+ init_completion(&req->completion);
+
+ adc_add_request_to_queue(pcf, req);
+
+ wait_for_completion(&req->completion);
+ result = req->result;
+ kfree(req);
+
+ return result;
+}
+
+static int adc_async_read(struct pcf50633_data *pcf, int mux, int avg,
+ void (*callback)(struct pcf50633_data *, int))
+{
+ struct adc_request *req;
+
+ req = kmalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ req->mux = mux;
+ req->avg = avg;
+ req->callback = callback;
+ req->request_type = ADC_REQUEST_TYPE_ASYNC;
+
+ adc_add_request_to_queue(pcf, req);
+
+ return 0;
+}
+
/*
* we get run to handle servicing the async notification from USB stack that
* we got enumerated and allowed to draw a particular amount of current
@@ -744,8 +802,9 @@ static void pcf50633_work_nobat(struct work_struct *work)
pcf->jiffies_last_bat_ins = jiffies;
/* figure out our charging stance */
- add_request_to_adc_queue(pcf, PCF50633_ADCC1_MUX_ADCIN1,
- PCF50633_ADCC1_AVERAGE_16);
+ (void)adc_async_read(pcf, PCF50633_ADCC1_MUX_ADCIN1,
+ PCF50633_ADCC1_AVERAGE_16,
+ configure_pmu_for_charger);
goto bail;
}
@@ -768,6 +827,7 @@ static void pcf50633_work(struct work_struct *work)
u_int8_t pcfirq[5];
int ret;
int tail;
+ struct adc_request *req;
mutex_lock(&pcf->working_lock);
pcf->working = 1;
@@ -873,8 +933,9 @@ static void pcf50633_work(struct work_struct *work)
}
/* figure out our initial charging stance */
- add_request_to_adc_queue(pcf, PCF50633_ADCC1_MUX_ADCIN1,
- PCF50633_ADCC1_AVERAGE_16);
+ (void)adc_async_read(pcf, PCF50633_ADCC1_MUX_ADCIN1,
+ PCF50633_ADCC1_AVERAGE_16,
+ configure_pmu_for_charger);
pcf->coldplug_done = 1;
}
@@ -912,8 +973,9 @@ static void pcf50633_work(struct work_struct *work)
PCF50633_FEAT_MBC, PMU_EVT_USB_INSERT);
msleep(500); /* debounce, allow to see any ID resistor */
/* completion irq will figure out our charging stance */
- add_request_to_adc_queue(pcf, PCF50633_ADCC1_MUX_ADCIN1,
- PCF50633_ADCC1_AVERAGE_16);
+ (void)adc_async_read(pcf, PCF50633_ADCC1_MUX_ADCIN1,
+ PCF50633_ADCC1_AVERAGE_16,
+ configure_pmu_for_charger);
}
if (pcfirq[0] & PCF50633_INT1_USBREM &&
!(pcfirq[0] & PCF50633_INT1_USBINS)) {
@@ -938,8 +1000,9 @@ static void pcf50633_work(struct work_struct *work)
pcf->last_curlim_set = 0;
/* completion irq will figure out our charging stance */
- add_request_to_adc_queue(pcf, PCF50633_ADCC1_MUX_ADCIN1,
- PCF50633_ADCC1_AVERAGE_16);
+ (void)adc_async_read(pcf, PCF50633_ADCC1_MUX_ADCIN1,
+ PCF50633_ADCC1_AVERAGE_16,
+ configure_pmu_for_charger);
}
}
if (pcfirq[0] & PCF50633_INT1_ALARM) {
@@ -1080,21 +1143,17 @@ static void pcf50633_work(struct work_struct *work)
tail = pcf->adc_queue_tail;
pcf->adc_queue_tail = (pcf->adc_queue_tail + 1) &
(MAX_ADC_FIFO_DEPTH - 1);
+ req = pcf->adc_queue[tail];
+ WARN_ON(!pcf);
- switch (pcf->adc_queue_mux[tail]) {
- case PCF50633_ADCC1_MUX_BATSNS_RES: /* battery voltage */
- pcf->flag_bat_voltage_read = async_adc_complete(pcf);
- break;
- case PCF50633_ADCC1_MUX_ADCIN1: /* charger type */
- pcf->charger_adc_result_raw = async_adc_complete(pcf);
- pcf->charger_type = interpret_charger_type_from_adc(
- pcf, pcf->charger_adc_result_raw);
- configure_pmu_for_charger(pcf, pcf->charger_type);
- break;
- default:
- async_adc_complete(pcf);
- break;
+ if (req->request_type == ADC_REQUEST_TYPE_ASYNC) {
+ req->callback(pcf, adc_read_result(pcf));
+ kfree(req);
+ } else {
+ req->result = adc_read_result(pcf);
+ complete(&req->completion);
}
+
trigger_next_adc_job_if_any(pcf);
}
if (pcfirq[2] & PCF50633_INT3_ONKEY1S) {
@@ -1271,22 +1330,17 @@ static u_int8_t battvolt_scale(u_int16_t battvolt)
u_int16_t pcf50633_battvolt(struct pcf50633_data *pcf)
{
- int count = 10;
+ int ret;
- pcf->flag_bat_voltage_read = -1;
- add_request_to_adc_queue(pcf, PCF50633_ADCC1_MUX_BATSNS_RES,
+ ret = adc_sync_read(pcf, PCF50633_ADCC1_MUX_BATSNS_RES,
PCF50633_ADCC1_AVERAGE_16);
- while ((count--) && (pcf->flag_bat_voltage_read < 0))
- msleep(1);
+ if (ret < 0)
+ return ret;
- if (count < 0) { /* timeout somehow */
- DEBUGPC("pcf50633_battvolt timeout :-(\n");
- return -1;
- }
-
- return adc_to_batt_millivolts(pcf->flag_bat_voltage_read);
+ return adc_to_batt_millivolts(ret);
}
+
EXPORT_SYMBOL_GPL(pcf50633_battvolt);
static ssize_t show_battvolt(struct device *dev, struct device_attribute *attr,
@@ -1918,7 +1972,7 @@ static ssize_t show_charger_type(struct device *dev,
int mode = pcf50633_reg_read(pcf, PCF50633_REG_MBCC7) &
PCF56033_MBCC7_USB_MASK;
return sprintf(buf, "%s mode %s\n",
- names_charger_type[pcf->charger_type],
+/*FIXME*/ names_charger_type[0],
names_charger_modes[mode]);
}
@@ -1947,8 +2001,14 @@ static ssize_t show_charger_adc(struct device *dev,
{
struct i2c_client *client = to_i2c_client(dev);
struct pcf50633_data *pcf = i2c_get_clientdata(client);
+ int result;
+
+ result = adc_sync_read(pcf, PCF50633_ADCC1_MUX_ADCIN1,
+ PCF50633_ADCC1_AVERAGE_16);
+ if (result < 0)
+ return result;
- return sprintf(buf, "%d\n", pcf->charger_adc_result_raw);
+ return sprintf(buf, "%d\n", result);
}
static DEVICE_ATTR(charger_adc, 0444, show_charger_adc, NULL);
