On Mon, 2020-01-13 at 21:57 +0100, Lars-Peter Clausen wrote:
> [External]
>
> 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.
We've actually [recently] had a discussion about this internally regarding
the 'indio_dev->name'.
Maybe it's a good time to ask here (now).
A lot of our userspace stuff have been searching IIO devices via the 'name'
field in sysfs, which is the name assigned here.
That creates a problem when you have multiple devices with the same driver.
Which is why, one
So, then some questions would be:
Is a searching for IIO devices [in userspace] based on IIO device-name not
recommended? If not, what would be? Or what would be a better idea?
The ABI reads [hopefully I pulled up the right field]:
What: /sys/bus/iio/devices/iio:deviceX/name
KernelVersion: 2.6.35
Contact: linux-iio@xxxxxxxxxxxxxxx
Description:
Description of the physical chip / device for device X.
Typically a part number.
The text in description is a bit open to interpretation, so I can't make an
assessment of what is correct.
In case there was a discussion about this, sorry for repeating some things
now.
>
> > + indio_dev->info = &hmc425a_info;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + return devm_iio_device_register(&pdev->dev, indio_dev);
> > +}
