Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

Hello Lee,

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

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

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. 

Best Regards,
	Matti Vaittinen