From bd8ef10261d7ae92ad2b4925afd2b56f46175c47 Mon Sep 17 00:00:00 2001 From: Paul Fertser Date: Tue, 28 Jul 2009 00:58:48 +0400 Subject: mfd: revise locking for pcf50633 ADC Current implementation is prone to races, this patch attempts to remove all but one (in pcf50633_adc_sync_read). The idea is that we need to guard the queue access only on inserting and removing items. If we insert and there're no more items in the queue it means that the last irq already happened and we need to trigger ADC manually. If not, then the next conversion will be triggered by the irq handler upon completion of the previous. Signed-off-by: Paul Fertser Signed-off-by: Samuel Ortiz --- drivers/mfd/pcf50633-adc.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/mfd/pcf50633-adc.c') diff --git a/drivers/mfd/pcf50633-adc.c b/drivers/mfd/pcf50633-adc.c index c2d05becfa9..3d31e97d6a4 100644 --- a/drivers/mfd/pcf50633-adc.c +++ b/drivers/mfd/pcf50633-adc.c @@ -73,15 +73,10 @@ static void trigger_next_adc_job_if_any(struct pcf50633 *pcf) struct pcf50633_adc *adc = __to_adc(pcf); int head; - mutex_lock(&adc->queue_mutex); - head = adc->queue_head; - if (!adc->queue[head]) { - mutex_unlock(&adc->queue_mutex); + if (!adc->queue[head]) return; - } - mutex_unlock(&adc->queue_mutex); adc_setup(pcf, adc->queue[head]->mux, adc->queue[head]->avg); } @@ -99,16 +94,17 @@ adc_enqueue_request(struct pcf50633 *pcf, struct pcf50633_adc_request *req) if (adc->queue[tail]) { mutex_unlock(&adc->queue_mutex); + dev_err(pcf->dev, "ADC queue is full, dropping request\n"); return -EBUSY; } adc->queue[tail] = req; + if (head == tail) + trigger_next_adc_job_if_any(pcf); adc->queue_tail = (tail + 1) & (PCF50633_MAX_ADC_FIFO_DEPTH - 1); mutex_unlock(&adc->queue_mutex); - trigger_next_adc_job_if_any(pcf); - return 0; } @@ -124,6 +120,7 @@ pcf50633_adc_sync_read_callback(struct pcf50633 *pcf, void *param, int result) int pcf50633_adc_sync_read(struct pcf50633 *pcf, int mux, int avg) { struct pcf50633_adc_request *req; + int err; /* req is freed when the result is ready, in interrupt handler */ req = kzalloc(sizeof(*req), GFP_KERNEL); @@ -136,9 +133,13 @@ int pcf50633_adc_sync_read(struct pcf50633 *pcf, int mux, int avg) req->callback_param = req; init_completion(&req->completion); - adc_enqueue_request(pcf, req); + err = adc_enqueue_request(pcf, req); + if (err) + return err; + wait_for_completion(&req->completion); + /* FIXME by this time req might be already freed */ return req->result; } EXPORT_SYMBOL_GPL(pcf50633_adc_sync_read); @@ -159,9 +160,7 @@ int pcf50633_adc_async_read(struct pcf50633 *pcf, int mux, int avg, req->callback = callback; req->callback_param = callback_param; - adc_enqueue_request(pcf, req); - - return 0; + return adc_enqueue_request(pcf, req); } EXPORT_SYMBOL_GPL(pcf50633_adc_async_read); @@ -184,7 +183,7 @@ static void pcf50633_adc_irq(int irq, void *data) struct pcf50633_adc *adc = data; struct pcf50633 *pcf = adc->pcf; struct pcf50633_adc_request *req; - int head; + int head, res; mutex_lock(&adc->queue_mutex); head = adc->queue_head; @@ -199,12 +198,13 @@ static void pcf50633_adc_irq(int irq, void *data) adc->queue_head = (head + 1) & (PCF50633_MAX_ADC_FIFO_DEPTH - 1); + res = adc_result(pcf); + trigger_next_adc_job_if_any(pcf); + mutex_unlock(&adc->queue_mutex); - req->callback(pcf, req->callback_param, adc_result(pcf)); + req->callback(pcf, req->callback_param, res); kfree(req); - - trigger_next_adc_job_if_any(pcf); } static int __devinit pcf50633_adc_probe(struct platform_device *pdev) -- cgit v1.2.3 From 86b24e9fdd89f7cc8d3ea2c0a0b0fa733f2747ae Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Wed, 7 Oct 2009 02:49:34 +0200 Subject: mfd: pcf50633: Fix memleak in pcf50633_client_dev_register Since platform_device_add_data copies the passed data, the allocated subdev_pdata is never freed. A simple fix would be to either free subdev_pdata or put it onto the stack. But since the pcf50633 child devices can rely on beeing children of the pcf50633 core device it's much more elegant to get access to pcf50633 core structure through that link. This allows to get completly rid of pcf5033_subdev_pdata. Signed-off-by: Lars-Peter Clausen --- drivers/mfd/pcf50633-adc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/mfd/pcf50633-adc.c') diff --git a/drivers/mfd/pcf50633-adc.c b/drivers/mfd/pcf50633-adc.c index 3d31e97d6a4..6d2e8466df1 100644 --- a/drivers/mfd/pcf50633-adc.c +++ b/drivers/mfd/pcf50633-adc.c @@ -209,17 +209,16 @@ static void pcf50633_adc_irq(int irq, void *data) static int __devinit pcf50633_adc_probe(struct platform_device *pdev) { - struct pcf50633_subdev_pdata *pdata = pdev->dev.platform_data; struct pcf50633_adc *adc; adc = kzalloc(sizeof(*adc), GFP_KERNEL); if (!adc) return -ENOMEM; - adc->pcf = pdata->pcf; + adc->pcf = dev_to_pcf50633(pdev->dev.parent); platform_set_drvdata(pdev, adc); - pcf50633_register_irq(pdata->pcf, PCF50633_IRQ_ADCRDY, + pcf50633_register_irq(adc->pcf, PCF50633_IRQ_ADCRDY, pcf50633_adc_irq, adc); mutex_init(&adc->queue_mutex); -- cgit v1.2.3