[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bb33aeb0-7134-45ec-8536-47c524ec9e4b@kernel.org>
Date: Tue, 13 Aug 2024 11:46:32 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: "Shen Jianping (ME-SE/EAD2)" <Jianping.Shen@...bosch.com>,
"jic23@...nel.org" <jic23@...nel.org>, "lars@...afoo.de" <lars@...afoo.de>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"dima.fedrau@...il.com" <dima.fedrau@...il.com>,
"marcelo.schmitt1@...il.com" <marcelo.schmitt1@...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>,
"Lorenz Christian (ME-SE/EAD2)" <Christian.Lorenz3@...bosch.com>,
"Frauendorf Ulrike (ME/PJ-SW3)" <Ulrike.Frauendorf@...bosch.com>,
"Dolde Kai (ME-SE/PAE-A3)" <Kai.Dolde@...bosch.com>
Subject: Re: [PATCH v2 2/2] iio: imu: smi240: imu driver
On 13/08/2024 11:41, Shen Jianping (ME-SE/EAD2) wrote:
> Hi
>
> On 09/08/2024 13:16, Jianping.Shen@...bosch.com wrote:
>> From: "Shen Jianping (ME-SE/EAD2)" <Jianping.Shen@...bosch.com>
>>
>> iio: imu: smi240: driver improvements
>
> ?????
> Did not get your point, what is wrong here? how it shall be?
See submitting patches. This does not match your commit at all. I do not
see any driver improvements done here. If so, please list all your
improvements against existing kernel driver.
>
>> Signed-off-by: Shen Jianping (ME-SE/EAD2) <Jianping.Shen@...bosch.com>
>> ---
>>
>
>
> ...
>
>> + ret = regmap_read(data->regmap, SMI240_CHIP_ID_REG, &response);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Read chip id failed\n");
>> +
>> + if (response != SMI240_CHIP_ID)
>> + dev_info(dev, "Unknown chip id: 0x%04x\n", response);
>> +
>> + ret = smi240_init(data);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "Device initialization failed\n");
>> +
>> + indio_dev->channels = smi240_channels;
>> + indio_dev->num_channels = ARRAY_SIZE(smi240_channels);
>> + indio_dev->name = "smi240";
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> + indio_dev->info = &smi240_info;
>> +
>> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>> + iio_pollfunc_store_time,
>> + smi240_trigger_handler, NULL);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "Setup triggered buffer failed\n");
>> +
>> + ret = devm_iio_device_register(dev, indio_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Register IIO device failed\n");
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(smi240_core_probe);
>> +
>> +MODULE_AUTHOR("Markus Lochmann <markus.lochmann@...bosch.com>");
>> +MODULE_AUTHOR("Stefan Gutmann <stefan.gutmann@...bosch.com>");
>> +MODULE_DESCRIPTION("Bosch SMI240 driver"); MODULE_LICENSE("Dual
>> +BSD/GPL");
>
> Hm? How many modules do you have here? What are their names?
>
> We have one module, named "Bosch SMI240 driver". Any problem here?
Yes, you put MODULE_* to how many files? Two? Three? Why is it needed
everywhere?
>
>
>
>> +
>> +static const struct spi_device_id smi240_spi_id[] = { { "smi240", 0
>> +}, {} };
>
> Don't wrap it.
>
> We don't , git send-mail did it automatically for us.
>
>
>> +MODULE_DEVICE_TABLE(spi, smi240_spi_id);
>> +
>> +static const struct of_device_id smi240_of_match[] = {
>> + { .compatible = "bosch,smi240" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, smi240_of_match);
>> +
>> +static struct spi_driver smi240_spi_driver = {
>> + .probe = smi240_spi_probe,
>> + .id_table = smi240_spi_id,
>> + .driver = {
>> + .of_match_table = of_match_ptr(smi240_of_match),
>
> Why did it appear? You introduce now warnings.
>
> Did not get your point, why we introduce now warnings here ?
Fix your quoting. It's impossible to figure out what is here my quote
and what is yours.
Why? Test your code properly... Drop the of_match_ptr.
Best regards,
Krzysztof
Powered by blists - more mailing lists