diff mbox series

[v3,18/23] regulator: add s2dos05 regulator support

Message ID 20240618-starqltechn_integration_upstream-v3-18-e3f6662017ac@gmail.com
State New
Headers show
Series This is continued work on Samsung S9(SM-9600) starqltechn | expand

Commit Message

Dzmitry Sankouski June 18, 2024, 1:59 p.m. UTC 
S2dos05 has 1 buck and 4 LDO regulators, used for powering
panel/touchscreen.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
 MAINTAINERS                           |   1 +
 drivers/regulator/Kconfig             |   8 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/s2dos05-regulator.c | 362 ++++++++++++++++++++++++++++++++++
 4 files changed, 372 insertions(+)

Comments

Mark Brown June 18, 2024, 2:08 p.m. UTC | #1 
On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:

> index 000000000000..3c58a1bd2262
> --- /dev/null
> +++ b/drivers/regulator/s2dos05-regulator.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * s2dos05.c - Regulator driver for the Samsung s2dos05
> + *

Please make the entire comment a C++ one so things look more
intentional.

> +static int s2m_enable(struct regulator_dev *rdev)
> +{
> +	struct s2dos05_data *info = rdev_get_drvdata(rdev);
> +	struct regmap *regmap = info->regmap;
> +
> +	return regmap_update_bits(regmap, rdev->desc->enable_reg,
> +				  rdev->desc->enable_mask,
> +					rdev->desc->enable_mask);
> +}

Please use the generic regmap helpers rather than open coding them.

> +	reg_np = of_get_child_by_name(dev->parent->of_node, "regulators");
> +	if (!reg_np) {
> +		dev_err(dev, "could not find regulators sub-node\n");
> +		return -EINVAL;
> +	}
> +
> +	err = of_regulator_match(dev, reg_np, rdata, rdev_num);
> +	of_node_put(reg_np);

Use of_match for this rather than open coding.

> +	s2dos05 = devm_kzalloc(dev, sizeof(struct s2dos05_data),
> +				GFP_KERNEL);

> +	rdata = kcalloc(rdev_num, sizeof(*rdata), GFP_KERNEL);
> +	if (!rdata)
> +		return -ENOMEM;

Mixing devm_ and regular allocations seems likely to go wrong, please be
consistent.
Dzmitry Sankouski June 19, 2024, 3:49 p.m. UTC | #2 
вт, 18 июн. 2024 г. в 17:08, Mark Brown <broonie@kernel.org>:
>
> On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:
>
> > index 000000000000..3c58a1bd2262
> > --- /dev/null
> > +++ b/drivers/regulator/s2dos05-regulator.c
> > @@ -0,0 +1,362 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * s2dos05.c - Regulator driver for the Samsung s2dos05
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
Do you mean enclosing the first line (license identifier) in /* */
style comment?
Mark Brown June 19, 2024, 3:52 p.m. UTC | #3 
On Wed, Jun 19, 2024 at 06:49:06PM +0300, Dzmitry Sankouski wrote:
> вт, 18 июн. 2024 г. в 17:08, Mark Brown <broonie@kernel.org>:
> > On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:

> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * s2dos05.c - Regulator driver for the Samsung s2dos05
> > > + *

> > Please make the entire comment a C++ one so things look more
> > intentional.

> Do you mean enclosing the first line (license identifier) in /* */
> style comment?

No, that would be a C comment.  Please use C++ style for the rest of the
header as well as the first line.
Krzysztof Kozlowski June 20, 2024, 4:08 p.m. UTC | #4 
On 18/06/2024 15:59, Dzmitry Sankouski wrote:
> S2dos05 has 1 buck and 4 LDO regulators, used for powering
> panel/touchscreen.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>


> diff --git a/drivers/regulator/s2dos05-regulator.c b/drivers/regulator/s2dos05-regulator.c
> new file mode 100644
> index 000000000000..3c58a1bd2262
> --- /dev/null
> +++ b/drivers/regulator/s2dos05-regulator.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * s2dos05.c - Regulator driver for the Samsung s2dos05
> + *
> + * Copyright (C) 2016 Samsung Electronics

Not happy. You upstream old issues. :( Please drop all junk Samsung code.

> + * Copyright (C) 2023 Dzmitry Sankouski <dsankouski@gmail.com>
> + *
> + */

...

> +
> +	return ret;
> +
> +err_data:
> +	devm_kfree(dev, (void *)s2dos05);

Why?

> +	kfree(s2dos05);

Please test this first. This is obviously wrong and having such trivial
issue makes me question what else is in this Samsung code.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index b53462684a30..bee700a5e648 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19910,6 +19910,7 @@  F:	Documentation/devicetree/bindings/regulator/samsung,s5m*.yaml
 F:	drivers/clk/clk-s2mps11.c
 F:	drivers/mfd/s2dos*.c
 F:	drivers/mfd/sec*.c
+F:	drivers/regulator/s2dos*.c
 F:	drivers/regulator/s2m*.c
 F:	drivers/regulator/s5m*.c
 F:	drivers/rtc/rtc-s5m.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index d333be2bea3b..d6d6f571a65d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1297,6 +1297,14 @@  config REGULATOR_RTQ2208
 	  and two ldos. It features wide output voltage range from 0.4V to 2.05V
 	  and the capability to configure the corresponding power stages.
 
+config REGULATOR_S2DOS05
+	tristate "SLSI S2DOS05 regulator"
+	depends on MFD_S2DOS_CORE || COMPILE_TEST
+	help
+	  This driver provides support for the voltage regulators of the S2DOS05.
+	  The S2DOS05 is a companion power management IC for the smart phones.
+	  The S2DOS05 has 4 LDOs and 1 BUCK outputs.
+
 config REGULATOR_S2MPA01
 	tristate "Samsung S2MPA01 voltage regulator"
 	depends on MFD_SEC_CORE || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index ba15fa5f30ad..80f889404597 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -151,6 +151,7 @@  obj-$(CONFIG_REGULATOR_RTMV20)	+= rtmv20-regulator.o
 obj-$(CONFIG_REGULATOR_RTQ2134) += rtq2134-regulator.o
 obj-$(CONFIG_REGULATOR_RTQ6752)	+= rtq6752-regulator.o
 obj-$(CONFIG_REGULATOR_RTQ2208) += rtq2208-regulator.o
+obj-$(CONFIG_REGULATOR_S2DOS05) += s2dos05-regulator.o
 obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
 obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
 obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
diff --git a/drivers/regulator/s2dos05-regulator.c b/drivers/regulator/s2dos05-regulator.c
new file mode 100644
index 000000000000..3c58a1bd2262
--- /dev/null
+++ b/drivers/regulator/s2dos05-regulator.c
@@ -0,0 +1,362 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * s2dos05.c - Regulator driver for the Samsung s2dos05
+ *
+ * Copyright (C) 2016 Samsung Electronics
+ * Copyright (C) 2023 Dzmitry Sankouski <dsankouski@gmail.com>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/samsung/s2dos-core.h>
+#include <linux/mfd/samsung/s2dos05.h>
+#include <linux/i2c.h>
+#include <linux/debugfs.h>
+
+struct s2dos05_data {
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static int s2m_enable(struct regulator_dev *rdev)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+
+	return regmap_update_bits(regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask,
+					rdev->desc->enable_mask);
+}
+
+static int s2m_disable_regmap(struct regulator_dev *rdev)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	u8 val;
+
+	if (rdev->desc->enable_is_inverted)
+		val = rdev->desc->enable_mask;
+	else
+		val = 0;
+
+	return regmap_update_bits(regmap, rdev->desc->enable_reg, rdev->desc->enable_mask,
+				   val);
+}
+
+static int s2m_is_enabled_regmap(struct regulator_dev *rdev)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(regmap, rdev->desc->enable_reg, &val);
+	if (ret < 0)
+		return ret;
+
+	if (rdev->desc->enable_is_inverted)
+		return (val & rdev->desc->enable_mask) == 0;
+	else
+		return (val & rdev->desc->enable_mask) != 0;
+}
+
+static int s2m_get_voltage_sel_regmap(struct regulator_dev *rdev)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(regmap, rdev->desc->vsel_reg, &val);
+	if (ret < 0)
+		return ret;
+
+	val &= rdev->desc->vsel_mask;
+
+	return val;
+}
+
+static int s2m_set_voltage_sel_regmap(struct regulator_dev *rdev,
+					unsigned int sel)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	int ret;
+
+	ret = regmap_update_bits(regmap, rdev->desc->vsel_reg, rdev->desc->vsel_mask,
+				sel);
+	if (ret < 0)
+		goto out;
+
+	if (rdev->desc->apply_bit)
+		ret = regmap_update_bits(regmap, rdev->desc->apply_reg,
+					 rdev->desc->apply_bit,
+					 rdev->desc->apply_bit);
+	return ret;
+out:
+	pr_warn("%s: failed to set voltage_sel_regmap\n", rdev->desc->name);
+	return ret;
+}
+
+static int s2m_set_voltage_sel_regmap_buck(struct regulator_dev *rdev,
+					unsigned int sel)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, rdev->desc->vsel_reg, sel);
+	if (ret < 0)
+		goto out;
+
+	if (rdev->desc->apply_bit)
+		ret = regmap_update_bits(regmap, rdev->desc->apply_reg,
+					 rdev->desc->apply_bit,
+					 rdev->desc->apply_bit);
+	return ret;
+out:
+	pr_warn("%s: failed to set voltage_sel_regmap\n", rdev->desc->name);
+	return ret;
+}
+
+static int s2m_set_voltage_time_sel(struct regulator_dev *rdev,
+				   unsigned int old_selector,
+				   unsigned int new_selector)
+{
+	int old_volt, new_volt;
+
+	/* sanity check */
+	if (!rdev->desc->ops->list_voltage)
+		return -EINVAL;
+
+	old_volt = rdev->desc->ops->list_voltage(rdev, old_selector);
+	new_volt = rdev->desc->ops->list_voltage(rdev, new_selector);
+
+	if (old_selector < new_selector)
+		return DIV_ROUND_UP(new_volt - old_volt, S2DOS05_RAMP_DELAY);
+
+	return 0;
+}
+
+static int s2m_set_active_discharge(struct regulator_dev *rdev,
+					bool enable)
+{
+	struct s2dos05_data *info = rdev_get_drvdata(rdev);
+	struct regmap *regmap = info->regmap;
+	int ret;
+	u8 val;
+
+	if (enable)
+		val = rdev->desc->active_discharge_on;
+	else
+		val = rdev->desc->active_discharge_off;
+
+	ret = regmap_update_bits(regmap, rdev->desc->active_discharge_reg,
+				rdev->desc->active_discharge_mask, val);
+	return ret;
+}
+
+static const struct regulator_ops s2dos05_ldo_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= s2m_is_enabled_regmap,
+	.enable			= s2m_enable,
+	.disable		= s2m_disable_regmap,
+	.get_voltage_sel	= s2m_get_voltage_sel_regmap,
+	.set_voltage_sel	= s2m_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= s2m_set_voltage_time_sel,
+	.set_active_discharge	= s2m_set_active_discharge,
+};
+
+static const struct regulator_ops s2dos05_buck_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= s2m_is_enabled_regmap,
+	.enable			= s2m_enable,
+	.disable		= s2m_disable_regmap,
+	.get_voltage_sel	= s2m_get_voltage_sel_regmap,
+	.set_voltage_sel	= s2m_set_voltage_sel_regmap_buck,
+	.set_voltage_time_sel	= s2m_set_voltage_time_sel,
+	.set_active_discharge	= s2m_set_active_discharge,
+};
+
+#define _BUCK(macro)	S2DOS05_BUCK##macro
+#define _buck_ops(num)	s2dos05_buck_ops##num
+
+#define _LDO(macro)	S2DOS05_LDO##macro
+#define _REG(ctrl)	S2DOS05_REG##ctrl
+#define _ldo_ops(num)	s2dos05_ldo_ops##num
+#define _MASK(macro)	S2DOS05_ENABLE_MASK##macro
+#define _TIME(macro)	S2DOS05_ENABLE_TIME##macro
+
+#define BUCK_DESC(_name, _id, _ops, m, s, v, e, em, t, a) {	\
+	.name		= _name,				\
+	.id		= _id,					\
+	.ops		= _ops,					\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= m,					\
+	.uV_step	= s,					\
+	.n_voltages	= S2DOS05_BUCK_N_VOLTAGES,		\
+	.vsel_reg	= v,					\
+	.vsel_mask	= S2DOS05_BUCK_VSEL_MASK,		\
+	.enable_reg	= e,					\
+	.enable_mask	= em,					\
+	.enable_time	= t,					\
+	.active_discharge_off = 0,				\
+	.active_discharge_on = S2DOS05_BUCK_FD_MASK,		\
+	.active_discharge_reg	= a,				\
+	.active_discharge_mask	= S2DOS05_BUCK_FD_MASK		\
+}
+
+#define LDO_DESC(_name, _id, _ops, m, s, v, e, em, t, a) {	\
+	.name		= _name,				\
+	.id		= _id,					\
+	.ops		= _ops,					\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= m,					\
+	.uV_step	= s,					\
+	.n_voltages	= S2DOS05_LDO_N_VOLTAGES,		\
+	.vsel_reg	= v,					\
+	.vsel_mask	= S2DOS05_LDO_VSEL_MASK,		\
+	.enable_reg	= e,					\
+	.enable_mask	= em,					\
+	.enable_time	= t,					\
+	.active_discharge_off = 0,				\
+	.active_discharge_on = S2DOS05_LDO_FD_MASK,		\
+	.active_discharge_reg	= a,				\
+	.active_discharge_mask	= S2DOS05_LDO_FD_MASK		\
+}
+
+static struct regulator_desc regulators[S2DOS05_REGULATOR_MAX] = {
+		/* name, id, ops, min_uv, uV_step, vsel_reg, enable_reg */
+		LDO_DESC("ldo1", _LDO(1), &_ldo_ops(), _LDO(_MIN1),
+			_LDO(_STEP1), _REG(_LDO1_CFG),
+			_REG(_EN), _MASK(_L1), _TIME(_LDO), _REG(_LDO1_CFG)),
+		LDO_DESC("ldo2", _LDO(2), &_ldo_ops(), _LDO(_MIN1),
+			_LDO(_STEP1), _REG(_LDO2_CFG),
+			_REG(_EN), _MASK(_L2), _TIME(_LDO), _REG(_LDO2_CFG)),
+		LDO_DESC("ldo3", _LDO(3), &_ldo_ops(), _LDO(_MIN2),
+			_LDO(_STEP1), _REG(_LDO3_CFG),
+			_REG(_EN), _MASK(_L3), _TIME(_LDO), _REG(_LDO3_CFG)),
+		LDO_DESC("ldo4", _LDO(4), &_ldo_ops(), _LDO(_MIN2),
+			_LDO(_STEP1), _REG(_LDO4_CFG),
+			_REG(_EN), _MASK(_L4), _TIME(_LDO), _REG(_LDO4_CFG)),
+		BUCK_DESC("buck1", _BUCK(1), &_buck_ops(), _BUCK(_MIN1),
+			_BUCK(_STEP1), _REG(_BUCK_VOUT),
+			_REG(_EN), _MASK(_B1), _TIME(_BUCK), _REG(_BUCK_CFG)),
+};
+
+static int s2dos05_pmic_dt_parse_pdata(struct device *dev,
+					struct of_regulator_match *rdata,
+					unsigned int rdev_num)
+{
+	struct device_node *reg_np;
+	int err;
+
+	reg_np = of_get_child_by_name(dev->parent->of_node, "regulators");
+	if (!reg_np) {
+		dev_err(dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	err = of_regulator_match(dev, reg_np, rdata, rdev_num);
+	of_node_put(reg_np);
+
+	return err;
+}
+
+static int s2dos05_pmic_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct s2dos_core *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct of_regulator_match *rdata = NULL;
+	struct s2dos05_data *s2dos05;
+	struct regulator_config config = { };
+	unsigned int rdev_num = ARRAY_SIZE(regulators);
+	int i;
+	int ret, err = 0;
+
+	s2dos05 = devm_kzalloc(dev, sizeof(struct s2dos05_data),
+				GFP_KERNEL);
+	if (!s2dos05) {
+		ret = -ENOMEM;
+		goto err_data;
+	}
+	platform_set_drvdata(pdev, s2dos05);
+
+	rdata = kcalloc(rdev_num, sizeof(*rdata), GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	for (i = 0; i < rdev_num; i++)
+		rdata[i].name = regulators[i].name;
+
+	err = s2dos05_pmic_dt_parse_pdata(dev, rdata, rdev_num);
+	if (err < 0) {
+		dev_err(dev, "Failed to parse regulators device of_node\n");
+		goto err_data;
+	}
+
+	s2dos05->regmap = iodev->regmap;
+	s2dos05->dev = dev;
+
+	for (i = 0; i < rdev_num; i++) {
+		struct regulator_dev *regulator;
+
+		config.init_data = rdata[i].init_data;
+		config.of_node = rdata[i].of_node;
+		config.dev = dev;
+		config.driver_data = s2dos05;
+		regulator = devm_regulator_register(&pdev->dev,
+						&regulators[i], &config);
+		if (IS_ERR(regulator)) {
+			ret = PTR_ERR(regulator);
+			dev_err(&pdev->dev, "regulator init failed for %d\n",
+				i);
+			goto out;
+		}
+	}
+
+out:
+	kfree(rdata);
+
+	return ret;
+
+err_data:
+	devm_kfree(dev, (void *)s2dos05);
+	kfree(s2dos05);
+
+	return ret;
+}
+
+static const struct platform_device_id s2dos05_pmic_id[] = {
+	{ "s2dos05-regulator" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, s2dos05_pmic_id);
+
+static struct platform_driver s2dos05_platform_driver = {
+	.driver = {
+		.name = "s2dos05",
+	},
+	.probe = s2dos05_pmic_probe,
+	.id_table = s2dos05_pmic_id,
+};
+module_platform_driver(s2dos05_platform_driver);
+
+MODULE_AUTHOR("Dzmitry Sankouski <dsankouski@gmail.com>");
+MODULE_DESCRIPTION("SAMSUNG s2dos05 Regulator Driver");
+MODULE_LICENSE("GPL");