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

[Vaittinen, Matti]

Hi deee Ho Lee,

On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:
> Hello Lee,
> 
> On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> > On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> > 
> > > Hello Lee,
> > > 
> > > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > > 
> > > > > 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@xxxxxxxxxxxxxxxxx>
> > > > > ---
> > > > >  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.
> > 
> > > 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.
> 
> > > 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.
> > 
> > > 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).

Best Regards
	Matti Vaittinen