Re: [PATCH 1/3] iio: amplifiers: hmc425a: Add support for HMC425A step attenuator with gpio interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/13/20 3:15 PM, Beniamin Bia wrote:
[...]
> +static int hmc425a_write(struct iio_dev *indio_dev, u32 value)
> +{
> +	struct hmc425a_state *st = iio_priv(indio_dev);
> +	int i, *values;
> +
> +	values = kmalloc_array(st->chip_info->num_gpios, sizeof(int),
> +			       GFP_KERNEL);
> +	if (!values)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < st->chip_info->num_gpios; i++)
> +		values[i] = (value >> i) & 1;
> +
> +	gpiod_set_array_value_cansleep(st->gpios->ndescs, st->gpios->desc,
> +				       values);

This API got changed a while ago in upstream, see
https://github.com/analogdevicesinc/linux/commit/b9762bebc6332b40c33e03dea03e30fa12d9e3ed

> +	kfree(values);
> +	return 0;
> +}
[...]
> +static int hmc425a_probe(struct platform_device *pdev)
> +{
[...]
> +
> +	platform_set_drvdata(pdev, indio_dev);

drvdata is never accessed, no need to set it.

> +	mutex_init(&st->lock);
> +
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->name = np->name;

I know ADI likes to do this in its non upstream drivers, but the above
is not IIO ABI compliant. The name is supposed to identify the type of
the device, which means for this driver should be static "hmc425a".
Maybe consider adding a field to the hmc425a_chip_info for this.

> +	indio_dev->info = &hmc425a_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	return devm_iio_device_register(&pdev->dev, indio_dev);
> +}




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux