Re: [PATCH v3 03/10] mfd: support ROHM BD96801 PMIC core

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 6/13/24 19:15, Lee Jones wrote:
> On Tue, 04 Jun 2024, Matti Vaittinen wrote:
> 
>> The ROHM BD96801 PMIC is highly customizable automotive grade PMIC
>> which integrates regulator and watchdog funtionalities.
>>
>> Provide INTB IRQ and register accesses for regulator/watchdog drivers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Changelog:
>> v2 =>:
>> - No changes
>>
>> v1 => v2:
>> - Drop unused enum
>> - Improve error prints
>> - improve comments
>>
>>   RFCv2 => v1:
>> - drop ERRB interrupts (for now)
>> - bd96801: Unlock registers in core driver
>>
>> Changelog: RFCv1 => RFCv2
>> - Work-around the IRQ domain name conflict
>> - Add watchdog IRQ
>> - Various styling fixes based on review by Lee
>> ---
>>   drivers/mfd/Kconfig              |  13 ++
>>   drivers/mfd/Makefile             |   1 +
>>   drivers/mfd/rohm-bd96801.c       | 273 +++++++++++++++++++++++++++++++
>>   include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
>>   include/linux/mfd/rohm-generic.h |   1 +
>>   5 files changed, 503 insertions(+)
>>   create mode 100644 drivers/mfd/rohm-bd96801.c
>>   create mode 100644 include/linux/mfd/rohm-bd96801.h
> 
> Pretty nice.  Uses generic interfaces.  Just a couple of nits.

Thanks :)

I'll try to send the next version of this series Today. I will only 
include the patches 1-6, which I believe are pretty much good to be 
merged (but the watchdog has not yet been acked by Guenter so no 
guarantees), and which should result a working driver for many of the 
predicted use-cases. Then I plan to be mostly off of my computer for a 
few weeks. (I may do some very minor fixes.)

The 7-10 will have to wait until the irqdomain name conflict can be 
reasonably resolved. I plan to return to this when I'm back from my "off 
time", and when we see what direction the irqdomain work by Herve has 
evolved :)

...

> 
>> +	{
>> +		.name = "bd96801-wdt",
>> +		.resources = wdg_intb_irqs,
>> +		.num_resources = ARRAY_SIZE(wdg_intb_irqs),
>> +	}, {
>> +		.name = "bd96801-pmic",
> 
> I thought this was the PMIC?
> 
> What is this device?  Regulators?
> 
> "bd96801-regulator"?

Yep. Thanks!

> 
>> +		.resources = regulator_intb_irqs,
>> +		.num_resources = ARRAY_SIZE(regulator_intb_irqs),
>> +	},
>> +};

Yours,
	-- Matti

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

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