linux-kernel - RE: [PATCH v5 3/3] drivers: iio: adc: add support for ad777x family

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <DM6PR03MB4315306944DE2E5E8CD3B236F36A2@DM6PR03MB4315.namprd03.prod.outlook.com>
Date: Thu, 26 Sep 2024 13:41:57 +0000
From: "Nechita, Ramona" <Ramona.Nechita@...log.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: Lars-Peter Clausen <lars@...afoo.de>,
        "Tanislav, Cosmin"
	<Cosmin.Tanislav@...log.com>,
        "Hennerich, Michael"
	<Michael.Hennerich@...log.com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, "Sa,
 Nuno" <Nuno.Sa@...log.com>,
        Andy Shevchenko <andy@...nel.org>,
        David Lechner
	<dlechner@...libre.com>,
        "Schmitt, Marcelo" <Marcelo.Schmitt@...log.com>,
        Olivier Moysan <olivier.moysan@...s.st.com>,
        Dumitru Ceclan
	<mitrutzceclan@...il.com>,
        Matteo Martelli <matteomartelli3@...il.com>,
        AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
        Alisa-Dariana Roman <alisadariana@...il.com>,
        Ivan Mikhaylov
	<fr0st61te@...il.com>,
        Mike Looijmans <mike.looijmans@...ic.nl>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH v5 3/3] drivers: iio: adc: add support for ad777x family

Hello all,

There is another thing that I forgot to mention inline with my last email.

> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of 
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only the SDO data streaming mode. SPI 
> communication is used alternatively for accessing registers and 
> streaming data. Register access are protected by crc8.
> 
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@...log.com>
Hi Ramona,

....

>> +static int ad7779_probe(struct spi_device *spi) {
>> +	struct iio_dev *indio_dev;
>> +	struct ad7779_state *st;
>> +	struct gpio_desc *reset_gpio, *start_gpio;
>> +	int ret;
>> +
>> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	st = iio_priv(indio_dev);
>> +
>> +	st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
>> +	if (IS_ERR(st->mclk))
>> +		return PTR_ERR(st->mclk);
>> +
>> +	if (!spi->irq)
>> +		return dev_err_probe(&spi->dev, ret,
>> +				     "DRDY irq not present\n");
>> +
>> +	reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
>> +	if (IS_ERR(reset_gpio))
>> +		return PTR_ERR(reset_gpio);
>> +
>> +	start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
>> +	if (IS_ERR(start_gpio))
>> +		return PTR_ERR(start_gpio);
>> +
>> +	crc8_populate_msb(ad7779_crc8_table, AD7779_CRC8_POLY);
>> +	st->spi = spi;
>> +
>> +	st->chip_info = spi_get_device_match_data(spi);
>> +	if (!st->chip_info)
>> +		return -ENODEV;
>> +
>> +	ret = ad7779_reset(indio_dev, reset_gpio);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ad7779_powerup(st, start_gpio);
>> +	if (ret)
>> +		return ret;
>What powers the device down again if we hit an error?
>
>Probably need a devm_add_action_or_reset() or if it self powers down may a comment on that.

In the powerup function there are only some register writes and the start gpio is only a synchronization pulse (perhaps the name powerup is not very appropriate), 
would an action or reset be necessary in this case? Since the regulators are not used in the driver, should there be a function disabling them anyway?


Best Regards,
Ramona