Re: [PATCH v3 2/5] mfd: sec: Add support for S2MPS15 PMIC

From: Alim Akhtar
Date: Thu Oct 29 2015 - 00:18:05 EST

On 10/28/2015 07:21 PM, Lee Jones wrote:
On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:

W dniu 28.10.2015 o 17:46, Lee Jones pisze:
On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:

On 26.10.2015 23:34, Lee Jones wrote:
On Mon, 26 Oct 2015, Alim Akhtar wrote:

From: Thomas Abraham <thomas.ab@xxxxxxxxxxx>

Add support for S2MPS15 PMIC which is similar to S2MPS11 PMIC. The S2MPS15
PMIC supports 27 LDO regulators, 10 buck regulators, RTC, three 32.768KHz
clock outputs and battery charger. This patch adds initial support for
LDO and buck regulators of S2MPS15 device.

Signed-off-by: Thomas Abraham <thomas.ab@xxxxxxxxxxx>
Signed-off-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
[Alim: Added s2mps15_devs like rtc and clk and related changes]
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
drivers/mfd/sec-core.c | 31 +++++++
drivers/mfd/sec-irq.c | 8 ++
include/linux/mfd/samsung/core.h | 1 +
include/linux/mfd/samsung/s2mps15.h | 158 +++++++++++++++++++++++++++++++++++
4 files changed, 198 insertions(+)
create mode 100644 include/linux/mfd/samsung/s2mps15.h

I replied to the previous set and won't be reviewing this one until
all of the open points are solved.

The naming and compatibles used by the driver are confusing but how it
was at beginning. Beside the confusion, the names are correct:

1. Main mfd driver:
- compatible: samsung,s2mps1*-pmic
- driver name: sec_pmic

2. Regulator driver:
- no compatible (because it always searches for "regulators" subnode of
its parent... that is the convention/legacy behaviour)
- driver name: s2mps1*-pmic

I hope that explains your concerns.

It explains *why*, but doesn't ease my concerns in any way.

Unfortunately I've only just realised the disparity we have between
MFD and the Regulator subsystem, which is annoying because it's now
almost impossible to rectify.

We should have taken one of two views. Either a) The MFD is the PMIC
device which encompasses regulator control. In which case the MFD
and it's corresponding compatible string would be named *-pmic and the
regulator driver would be called *-regulator. Or b) The MFD could be
considered a normal MFD and be named after the model number, then the
regulator 'could' be named *-pmic.

However, with reference to b), how much other Power Management does
the regulator driver do besides control regulators? I suspect not
much. Therefore my preference would be for a). My second choice
would be a mixuture of the two where nothing gets named *-pmic. The
last option on my list would be the current situation where we seem to
be calling both the MFD (PMIC) itself and the Regulator driver
*-pmic, which is not good.

Starting from the description of device-family. This is called "Power
Management IC" but it is rather a "Power Deliver/Distribute IC". There
isn't any logic inside except enable/disable/configure/set low power
mode for regulators.

However in the same time the IC comes (always) with:
- 32kHz clocks,
- RTC,
- backup battery charger (no driver for it),
- reset for SoC,
- shutdown on thermal alert (also no driver for this control).

The solution a) seems fine to me. Make sense and it looks entirely
backward compatible - only driver names will be modified.

Perfect solution from my PoV.

Thanks Krzysztof.

Thanks Jones/Krzysztof as always.
Will update the patches.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at