Re: [PATCH v2 2/2] mfd: rohm-bd71828: Add power off functionality

From: Matti Vaittinen
Date: Thu Mar 28 2024 - 01:15:28 EST


Morning Andreas,

On 3/28/24 00:02, Andreas Kemnade wrote:
Hi Matti,

On Wed, 27 Mar 2024 16:11:36 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:

On 3/27/24 15:04, Andreas Kemnade wrote:
Hi,

On Wed, 27 Mar 2024 09:32:29 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
It's worth noting that there is another PMIC, BD71879, which, from the
driver software point of view, should be (almost?) identical to the
BD71828. I believe the BD71828 drivers should work with it as well - if
not out of the box, at least with very minor modifications.
Unfortunately I don't know products where the BD71879 is used or if it
is sold via distributors - so I don't know if adding a DT
compatible/chip type define for it would be beneficial.

yes, you already told we thet the BD71828 drivers are compatible with
the BD71879 and I am using the latter.
But that at least should be commented somewhere, so that
people do not raise questions, like: Do I have some strange board revision,
etc?
The most terse form to comment it is a separate dt compatible so we are
prepare any "almost identical" surprises.

I agree. Reason why I haven't done this already is that I don't always
(like in this case) know which of the variant are eventually sold. So,
it's balancing dance between adding compatibles for ICs that will never
been seen by large audience, and missing compatibles for some of the
variants.

This is also why I was interested in knowing which variant you had, and
where was it used.

I have found it in the Kobo Clara 2E ebook reader.
Kobo seems to switch from RC5T619 to BD71879.
The Kobo Nia rev C also has that one.
Kobo Libra 2 has several hardware revs out in the wild, some of them
with the BD71879.

Thanks for the info :) It's a shame we so rarely know where things we work for are used. I always find news like this interesting.

But yes, I think that as the BD71879 has obviously been found by a
community linux kernel user - it would make sense to add a compatible
for it!

Do you feel like adding the compatible 'rohm,bd71879' in
rohm,bd71828-pmic.yaml as part of this series(?)

Do we want a separate chip_type now? Or do we want to add it later if
we ever see a difference. My personal opinion is to wait until there is
really a need.

Using the BD71828 chip_id for BD71879 in the MFD driver is fine to me. A comment saying they seem "functionally equivalent" can be added to explain this choice.

If we do not need it, then it is a different series I think but sure
I will produce such a patch.

Great, thanks! I think it's clearer to have it as own patch, but I think it fits in the same series - what suits you best. (Don't know if Lee or DT peeps have different opinion.)

Yours,
-- Matti

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

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