linux-kernel - Re: [PATCH v2 2/2] iio: add mcp4728 I2C DAC driver

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] [day] [month] [year] [list]

Message-ID: <b82ee6d3-fa33-1e5a-be9d-7f13259dbcda@gmail.com>
Date:   Thu, 20 Jul 2023 14:16:24 +0200
From:   Andrea Collamati <andrea.collamati@...il.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        William Breathitt Gray <william.gray@...aro.org>,
        Angelo Dureghello <angelo.dureghello@...esys.com>,
        Fabio Estevam <festevam@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] iio: add mcp4728 I2C DAC driver


On 7/17/23 08:38, Krzysztof Kozlowski wrote:
> On 16/07/2023 23:26, Andrea Collamati wrote:
>> mcp4728 is a 12-bit quad channel DAC with I2C interface.
>>
>> support for:
>> * per-channel gain
>> * per-channel power state
>> * per-channel power down mode control
>> * per-channel vref selection internal/vdd
>> * store current state to on-chip EEPROM
>>
>> Signed-off-by: Andrea Collamati <andrea.collamati@...il.com>
>> ---
> What changed? Are you saying you ignored entire review you got?
I didn't ignored, but I didn't list the changes. I will look at other emails to understand how to write v2 v3 ... changes..Sorry.
>> drivers/iio/dac/Kconfig | 12 +
>> drivers/iio/dac/Makefile | 1 +
>> drivers/iio/dac/mcp4728.c | 635 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 648 insertions(+)
>> create mode 100644 drivers/iio/dac/mcp4728.c
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index 3acd9c3f388e..fa1516f6a285 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -389,6 +389,18 @@ config MCP4725
>> To compile this driver as a module, choose M here: the module
>> will be called mcp4725.
>> +config MCP4728
>> + tristate "MCP4728 DAC driver"
>> + depends on I2C
>> + help
>> + Say Y here if you want to build a driver for the Microchip
>> + MCP4728 quad channel, 12-bit digital-to-analog converter (DAC)
>> + with I2C interface.
>> +
>> + To compile this driver as a module, choose M here: the module
>> + will be called mcp4728.
>> +
>> +
> Why two blank lines?
>
>> config MCP4922
>> tristate "MCP4902, MCP4912, MCP4922 DAC driver"
>> depends on SPI
>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
>> index addd97a78838..5b2bac900d5a 100644
> ...
>
>> +
>> +static void mcp4728_remove(struct i2c_client *client)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> + iio_device_unregister(indio_dev);
>> + regulator_disable(data->vdd_reg);
>> +}
>> +
>> +static const struct i2c_device_id mcp4728_id[] = { { "mcp4728", MCP4728 }, {} };
>> +MODULE_DEVICE_TABLE(i2c, mcp4728_id);

Sorry.. I fixed but then I code formatter replace the old one. I will pay more attention..

besides clang-format and checkpatch are there any other tools to avoid these formatting errors?

Thank you

> Yeah, my feedback was ignored.
>
> That's not how it works. Anyway, I doubt that it should be a new driver.
>
> If Jonathan agrees to have new/duplicated drivers, then fine with me,
> but then don't ignore the comments. Instead:
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
Ok.
> Best regards,
> Krzysztof
>