lkml.org 
[lkml]   [2014]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/2] iio: adc: add driver for Rockchip saradc
On 07/14/2014 10:43 PM, Heiko Stübner wrote:

Looks really good overall.

[...]
> +static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&indio_dev->mlock);
> +
> + /* 8 clock periods as delay between power up and start cmd */
> + writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> +

It makes sense to call reinit_completion() here.

> + /* Select the channel to be used and trigger conversion */
> + writel(SARADC_CTRL_POWER_CTRL
> + | (chan->channel & SARADC_CTRL_CHN_MASK)
> + | SARADC_CTRL_IRQ_ENABLE,
> + info->regs + SARADC_CTRL);
> +
> + if (!wait_for_completion_timeout(&info->completion,
> + SARADC_TIMEOUT)) {
> + writel_relaxed(0, info->regs + SARADC_CTRL);
> + mutex_unlock(&indio_dev->mlock);
> + return -ETIMEDOUT;
> + }
> +
> + *val = info->last_val;
> + mutex_unlock(&indio_dev->mlock);
> + return IIO_VAL_INT;
[...]
> +static int rockchip_saradc_probe(struct platform_device *pdev)
> +{
> + struct rockchip_saradc *info = NULL;
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev = NULL;
> + struct resource *mem;
> + int ret;
> + int irq;
> + u32 rate;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> + info = iio_priv(indio_dev);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + info->regs = devm_request_and_ioremap(&pdev->dev, mem);

devm_request_and_ioremap() is deprecated an will be removed in the next
release[1]. Use devm_ioremap_resource(), note that devm_ioremap_resource()
returns a ERR_PTR instead of NULL on error.

[1] https://lkml.org/lkml/2014/6/11/26

> + if (!info->regs)
> + return -ENOMEM;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no irq resource?\n");
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq, rockchip_saradc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq %d\n", irq);
> + return ret;
> + }
> +
> + init_completion(&info->completion);

Since the completion is used in the interrupt callback it must be
initialized before the interrupt is requested.

> +
[...]
> + /* use a default of 1MHz for the converter clock */
> + ret = of_property_read_u32(np, "clock-frequency", &rate);
> + if (ret < 0)
> + rate = 1000000;

Should this really be in the devicetree or shouldn't this be user
configurable at runtime?

[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-07-20 14:01    [W:0.065 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site