Re: [PATCH v7 0/5] Support ROHM BU27034 ALS sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi deee Ho peeps!

On 3/31/23 15:40, Matti Vaittinen wrote:
Support ROHM BU27034 ALS sensor

This series adds support for ROHM BU27034 Ambient Light Sensor.

I have one word for all of you who worked to get the ROHM BU27034NUC driver working in upstream.

Meh.

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.

The issues of BU27034NUC were solved, and new model BU27034ANUC was developed and is available in the ROHM catalog.

I did also learn that this new model BU27034ANUC is _not_ functionally equivalent to the BU27034NUC. I am currently clarifying all the differences, and I have requested the HQ to send me a sample for driver development and verification work.

This far I've come to know at least following differences:

- The DATA2 (IR) channel is removed. So is the gain setting for it. This
  should very much simplify the gain logic.
- Some of the gains were removed.
- The 5ms integration time was removed. (The support of 5ms was severely
  limited on original BU27034NUC too so driver did not support that
  anyways).
- The light sensitivity curves had changed so the lux calculation will
  be changed.

One thing that has _not_ changed though is the part-id :rolleyes:

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.

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". Should that compatible be removed and a new one with "anuc"-suffix be added to denote the new sensor?

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...

Yours,
	-- Matti

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

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





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux