Re: [PATCH v7 0/5] Support ROHM BU27034 ALS sensor
From: Matti Vaittinen
Date: Mon Mar 11 2024 - 05:16:12 EST
On 3/9/24 19:50, Jonathan Cameron wrote:
On Mon, 4 Mar 2024 14:38:38 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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! ~~