Hello Lee,

Nice to see you are back :)

On Thu, 2021-01-14 at 10:00 +0000, Lee Jones wrote:

On Tue, 29 Dec 2020, Vaittinen, Matti wrote:

quoted

Hello Again peeps,

On Thu, 2020-12-17 at 12:04 +0200, Matti Vaittinen wrote:

quoted

On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:

quoted

Hello Lee,

On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:

quoted

On Fri, 27 Nov 2020, Vaittinen, Matti wrote:

quoted

Hello Lee,

On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:

quoted

On Mon, 23 Nov 2020, Matti Vaittinen wrote:

quoted

Add core support for ROHM BD9576MUF and BD9573MUF PMICs
which
are
mainly used to power the R-Car series processors.

Signed-off-by: Matti Vaittinen <
matti.vaittinen@fi.rohmeurope.com>
---
 drivers/mfd/Kconfig              |  11 ++++
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/rohm-bd9576.c        | 108
+++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd957x.h  |  59
+++++++++++++++++
 include/linux/mfd/rohm-generic.h |   2 +
 5 files changed, 181 insertions(+)
 create mode 100644 drivers/mfd/rohm-bd9576.c
 create mode 100644 include/linux/mfd/rohm-bd957x.h

Looks like a possible candidate for "simple-mfd-i2c".

Could you look into that please?

I must admit I didn't know about "simple-mfd-i2c". Good
thing
to
know
when working with simple devices :) Is this a new thing?

Yes, it's new.

quoted

I am unsure I understand the idea fully. Should users put
all
the
different regamp configs in this file and just add the
device
IDs
with
pointer to correct config? (BD9576 and BD9573 need volatile
ranges).
Also, does this mean each sub-device should have own node
and
own
compatible in DT to get correctly load and probed? I guess
this
would
need a buy-in from Rob too then.

You should describe the H/W in DT.

Yes. And it is described. But I've occasionally received
request
from
DT guys to add some properties directly to MFD node and not to
add
own
sub-node. This is what is done for example with the BD71837/47
clocks
-
there is no own node for clk - the clk properties are placed
directly
in MFD node (as was requested by Stephen and Rob back then - I
originally had own node for clk). I really have no clear view
on
when
things warrant for own subnode and when they don't - but as far
as
I
can see using simple-mfd-i2c forces one to always have a sub-
node /
device. Even just a empty node with nothing but the compatible
even
if
device does not need stuff from DT? Anyways, I think this is
nice
addition for simple drivers.

quoted

quoted

By the way - for uneducated eyes like mine this does not
look
like
it
has much to do with MFD as a device - here MFD reminds me
of a
simple-
bus on top of I2C.

This is for MFD devices where the parent does little more
than
create
a shared address space for child devices to operate on - like
yours.

quoted

Anyways, the BD9576 and BD9573 both have a few interrupts
for
OVD/UVD
conditions and I am expecting that I will be asked to
provide
the
regulator notifiers for those. Reason why I omitted the
IRQs
for
now is
that the HW is designed to keep the IRQ asserted for whole
error
duration so some delayed ack mechanism would be needed. I
would
like to
keep the door open for adding IRQs to MFD core.

You mean to add an IRQ Domain?

Yes. I planned to use regmap-irq and create irq chip in MFD
when
the
over / under voltage / temperature - notifications or watchdog
IRQs
are
needed. 

I am sorry if I have missed your reply. The ROHM email had
redirected
almost all patch emails to spam + I am not sure if some mails are
dropping :(

(I am considering moving to gmail - but I'd rather keep all mails
in
one system and I can't transfer work mail traffic to gmail... I
wonder
how others are managing the mails - which mail system you are
using?)

I think this series is now pending the decision how to proceed
with
MFD
part. If you still want me to start with "simple-mfd-i2c", then I
would
appreciate if you pointed me how you would like to see the regmap
configs added. Although I am quite positive this (eventually)
ends up
being more than what simple-mfd-i2c is intended for (because at
some
point people want to add the use of the interrupts).

Looking at this topic again. I kind of understand the idea of
combining
bunch of MFD drivers into one file. Many of the ROHM PMIC MFD
drivers
do provide same functionality. Regmap configs, regmap IRQ and MFD
cells. Some do also probe the device. So having own file for each
IC is
likely to not scale well when more devices are supported (and I do
hope
this will be the case also with the ROHM ICs).

What bugs me with the simple-mfd-i2c here is:
1. Requiring to have own compatibles for sub-devices (regulator and
WDG) to get them properly probed. (3 compatibles for 1 IC).
2. Requiring to have own DT node for WDG.
3. Supporting differences between BD9576 and BD9573 by having 6
compatibles for 2 ICs.
4. Adding interrupt support.

Linux sees each of these functions as separate devices which are
handled in different ways by isolated subsystems.  So yes, they each
require their own compatible string regardless of whether they share
the same physical piece of silicon or not.

My understanding is that this is exactly why we have MFD? To bring all
the functions under one multifunctional device and to handle things
which are common to all blocks (like IRQs). Besides, like you  know
(better than me) we don't need additional compatibles or dummy dt-nodes 
when MFD instantiates the sub-devices.

quoted

So ... How do you see adding BD9576/BD9573 MFD stuff in
BD9571/(BD9574)
MFD driver? The data structures (regmap configs, MFD cells, regmap
IRQ
portion when added) will be different but the functions and maybe
engineers looking at these may be common.

Is it just plain confusing to add core structures for technically
different ICs in same file - or is it way to avoid duplicating same
code in many files? I can try adding the BD9576/BD9573 to the
BD9571
core - or I can do resend this as is (rebased on 5.11). I can also
hack
this to be kicked by simple-mfd-i2c (although I have these strong
objections) - but I bet it will in the long run just lead to a sub-
optimal solution. When the BD9576/BD9573 logic blocks are re-used
in
some "non simple" designs and re-using the sub-drivers is needed
and/or
when IRQs are needed.

(BTW - I am currently working with BD71815/BD71817 - and after this
discussion I will add these in BD71828/BD71878 MFD core. I had
created
new MFD file for them but this discussion has been a nice kick to
the
better direction for me)

Everything will be a trade-off.

Yes. The BD71815 is in my opinion something we can and should handle
with same MFD driver as BD71828. 

The BD9576/BD9573 I'd still go with own MFD (as was in this series).
But if you as the maintainer absolutely require me to rework it in some
direction (regardless of my reasoning here) - please let me know. I
would like to get this in a shape that rest of the drivers can also be
accepted in-tree. I think this has been waiting for the decision what
to do with the MFD for a while now.

There will either be superflouous files or inflexible code.

You have to make the right decision for the driver and the subsystem.

I think I have explained what I think of but I need you to decide what
is best for the subsystem.


Best regards
	Matti Vaittinen