Received-SPF: pass (google.com: domain of linux-kernel+bounces-98595-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <02e2210d-9164-431e-8fe2-226cb1aa2d48@gmail.com>
Date: Mon, 11 Mar 2024 11:15:10 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 0/5] Support ROHM BU27034 ALS sensor
Content-Language: en-US, en-GB
To: Jonathan Cameron <jic23@kernel.org>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
 linux-iio@vger.kernel.org, Shreeya Patel <shreeya.patel@collabora.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 devicetree@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
 Paul Gazzillo <paul@pgazz.com>, Rob Herring <robh+dt@kernel.org>,
 Dmitry Osipenko <dmitry.osipenko@collabora.com>,
 linux-kernel@vger.kernel.org,
 Andy Shevchenko <andriy.shevchenko@linux.intel.com>
References: <cover.1680263956.git.mazziesaccount@gmail.com>
 <ff8d6d14-6b48-4347-8525-e05eeb9721ff@gmail.com>
 <20240309175056.3862630f@jic23-huawei>
From: Matti Vaittinen <mazziesaccount@gmail.com>
In-Reply-To: <20240309175056.3862630f@jic23-huawei>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 3/9/24 19:50, Jonathan Cameron wrote:
> On Mon, 4 Mar 2024 14:38:38 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:

>> I just found out that the BU27034 sensor which was developed when I
>> wrote this driver had some "manufacturing issues"... The full model
>> number was BU27034NUC. The has been cancelled, and, as far as I know, no
>> significant number of those were manufactured.
> 
> ouch. We all have some cancelled products in our history!
> When that happens I usually go eat cake and moan at anyone standing
> near by.

I like that approach :) Luckily, this was just a sensor. It was much 
more painful back in the Nokia when some of the BTS variants were 
cancelled. It flushed 'man years' of work instead of 'man months' :)

> At least this seems like there will be some direct use of
> the work done (sometimes you just have to list the things learnt along
> the way).

Yes! It wasn't all wasted effort!

>> One thing that has _not_ changed though is the part-id :rolleyes:
> 
> *sigh* Not even a version number?

No.

> Even unreleased / prototype parts should have
> different IDs if anything in the interface changed.

..tell me about it... Well, I tried to send feedback - but I am not 
convinced this is not happening again. I think I could fill a book with 
feedback which has had not been listened in the past - but who knows, 
occasionally feedback also works. So, we can keep trying. :)

>> My preferred approach would be to convert the in-tree bu27034 driver to
>> support this new variant. I think it makes sense because:
>> - (I expect) the amount of code to review will be much smaller this way
>>     than it would be if current driver was completely removed, and new one
>>     submitted.
>> - This change will not break existing users as there should not be such
>>     (judging the statement that the original BU27034NUC was cancelled
>>     before it was sold "en masse").
>>
>> It sure is possible to drop the existing driver and submit a new one
>> too, but I think it will be quite a bit more work with no strong benefits.
> 
> Agreed, modify the existing driver. Just needs a clear statement in
> patch descriptions that the original part is not expected to be in the wild.

ack.

>> I expect the rest of the information to be shared to me during the next
>> couple of days, and I hope I can start testing the driver immediately
>> when I get the HW.
>>
>> My question is, do you prefer the changes to be sent as one "support
>> BU27034ANUC patch, of would you rather see changes splitted to pieces
>> like: "adapt lux calculation to support BU27034ANUC", "remove obsolete
>> DATA2 channel", "remove unsupported gains"...? Furthermore, the DT
>> compatible was just rohm,bu27034 and did not include the ending "nuc".
> 
> Separate patches preferred for each feature / type of change. Mostly
> they'll hopefully be trivial to review.

I've drafted most of the changes and it seems they are more or less 
trivial. I've not yet received the hardware so the changes are 100% 
untested though.

>> Should that compatible be removed and a new one with "anuc"-suffix be
>> added to denote the new sensor?
> 
> Yes. The binding patch in particular will need a really clear statement
> that we believe there are none in products in the wild.

ack.

>> I am truly sorry for all the unnecessary reviewing and maintenance work
>> you guys did. I can assure you I didn't go through it for fun either -
>> even if the coding was fun :) I guess even the "upstream early" process
>> has it's weaknesses...
> 
> True enough. It's always 'interesting' to not know if / when a product
> you've upstreamed code for will launch.

Indeed. Almost as interesting as having patches for a new product in 
your "to be sent" - folder for 3 years waiting for the product to launch 
to get permission to send the patches... Don't we all love maintaining 
off-tree patches when we have that creeping feeling the patches will 
never be allowed to be sent...? Asking for a friend :rolleyes:

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~