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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5ae63616-5749-da51-b0b2-85cdcaa948f3@metafoo.de>
Date:   Mon, 13 Jan 2020 21:57:32 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Beniamin Bia <beniamin.bia@...log.com>, jic23@...nel.org
Cc:     Michael.Hennerich@...log.com, pmeerw@...erw.net,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        biabeniamin@...look.com, knaack.h@....de, robh+dt@...nel.org,
        mark.rutland@....com, devicetree@...r.kernel.org,
        Alexandru Ardelean <alexandru.ardelean@...log.com>
Subject: Re: [PATCH 1/3] iio: amplifiers: hmc425a: Add support for HMC425A
 step attenuator with gpio interface

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);
> +}