[PATCH 0/8] regulator: support Silergy SY8824C/SY8824E etc.

[PATCH 0/8] regulator: support Silergy SY8824C/SY8824E etc.

[Jisheng Zhang]

Add support for Silergy SY8824C/SY8824E/SY20276/SY20278 regulator.

Jisheng Zhang (8):
  regulator: add binding for the SY8824C voltage regulator
  regulator: add support for SY8824C regulator
  dt-bindings: sy8824x: Document SY8824E support
  regulator: sy8824x: add SY8824E support
  dt-bindings: sy8824x: Document SY20276 support
  regulator: sy8824x: add SY20276 support
  dt-bindings: sy8824x: Document SY20278 support
  regulator: sy8824x: add SY20278 support

 .../devicetree/bindings/regulator/sy8824x.txt |  24 ++
 drivers/regulator/Kconfig                     |   7 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/sy8824x.c                   | 231 ++++++++++++++++++
 4 files changed, 263 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/sy8824x.txt
 create mode 100644 drivers/regulator/sy8824x.c

-- 
2.23.0.rc1

[PATCH 1/8] regulator: add binding for the SY8824C voltage regulator

[Jisheng Zhang]

SY8824C is an I2C-controlled adjustable voltage regulator made by
Silergy Corp.

Add its device tree binding.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 .../devicetree/bindings/regulator/sy8824x.txt | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/sy8824x.txt

diff --git a/Documentation/devicetree/bindings/regulator/sy8824x.txt b/Documentation/devicetree/bindings/regulator/sy8824x.txt
new file mode 100644
index 000000000000..ff8d1af04f7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sy8824x.txt
@@ -0,0 +1,20 @@
+SY8824C Voltage regulator
+
+Required properties:
+- compatible: Must be "silergy,sy8824c"
+- reg: I2C slave address
+
+Any property defined as part of the core regulator binding, defined in
+./regulator.txt, can also be used.
+
+Example:
+
+	vcore: regulator@00 {
+		compatible = "silergy,sy8824c";
+		reg = <0x66>;
+		regulator-name = "vcore";
+		regulator-min-microvolt = <800000>;
+		regulator-max-microvolt = <1150000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
-- 
2.23.0.rc1

[PATCH 2/8] regulator: add support for SY8824C regulator

[Jisheng Zhang]

SY8824C is an I2C attached single output regulator made by Silergy Corp,
which is used on several Synaptics berlin platforms to control the
power supply of the ARM cores.

Add a driver for it.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/regulator/Kconfig   |   7 ++
 drivers/regulator/Makefile  |   1 +
 drivers/regulator/sy8824x.c | 192 ++++++++++++++++++++++++++++++++++++
 3 files changed, 200 insertions(+)
 create mode 100644 drivers/regulator/sy8824x.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index b57093d7c01f..e8093a909e85 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -906,6 +906,13 @@ config REGULATOR_SY8106A
 	help
 	  This driver supports SY8106A single output regulator.
 
+config REGULATOR_SY8824X
+	tristate "Silergy SY8824C regulator"
+	depends on I2C && (OF || COMPILE_TEST)
+	select REGMAP_I2C
+	help
+	  This driver supports SY8824C single output regulator.
+
 config REGULATOR_TPS51632
 	tristate "TI TPS51632 Power Regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index eef73b5a35a4..922bf975070f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -111,6 +111,7 @@ obj-$(CONFIG_REGULATOR_STM32_PWR) += stm32-pwr.o
 obj-$(CONFIG_REGULATOR_STPMIC1) += stpmic1_regulator.o
 obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
 obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
+obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
 obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
diff --git a/drivers/regulator/sy8824x.c b/drivers/regulator/sy8824x.c
new file mode 100644
index 000000000000..2d8a61ca6643
--- /dev/null
+++ b/drivers/regulator/sy8824x.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SY8824C regulator driver
+ *
+ * Copyright (C) 2019 Synaptics Incorporated
+ *
+ * Author: Jisheng Zhang <jszhang@kernel.org>
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define BUCK_EN		(1 << 7)
+#define MODE		(1 << 6)
+
+struct sy8824_config {
+	/* registers */
+	unsigned int vol_reg;
+	unsigned int mode_reg;
+	unsigned int enable_reg;
+	/* Voltage range and step(linear) */
+	unsigned int vsel_min;
+	unsigned int vsel_step;
+	unsigned int vsel_count;
+};
+
+struct sy8824_device_info {
+	struct device *dev;
+	struct regulator_desc desc;
+	struct regulator_init_data *regulator;
+	const struct sy8824_config *cfg;
+};
+
+static int sy8824_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct sy8824_device_info *di = rdev_get_drvdata(rdev);
+	const struct sy8824_config *cfg = di->cfg;
+
+	switch (mode) {
+	case REGULATOR_MODE_FAST:
+		regmap_update_bits(rdev->regmap, cfg->mode_reg, MODE, MODE);
+		break;
+	case REGULATOR_MODE_NORMAL:
+		regmap_update_bits(rdev->regmap, cfg->mode_reg, MODE, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static unsigned int sy8824_get_mode(struct regulator_dev *rdev)
+{
+	struct sy8824_device_info *di = rdev_get_drvdata(rdev);
+	const struct sy8824_config *cfg = di->cfg;
+	u32 val;
+	int ret = 0;
+
+	ret = regmap_read(rdev->regmap, cfg->mode_reg, &val);
+	if (ret < 0)
+		return ret;
+	if (val & MODE)
+		return REGULATOR_MODE_FAST;
+	else
+		return REGULATOR_MODE_NORMAL;
+}
+
+static const struct regulator_ops sy8824_regulator_ops = {
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.map_voltage = regulator_map_voltage_linear,
+	.list_voltage = regulator_list_voltage_linear,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.set_mode = sy8824_set_mode,
+	.get_mode = sy8824_get_mode,
+};
+
+static int sy8824_regulator_register(struct sy8824_device_info *di,
+			struct regulator_config *config)
+{
+	struct regulator_desc *rdesc = &di->desc;
+	const struct sy8824_config *cfg = di->cfg;
+	struct regulator_dev *rdev;
+
+	rdesc->name = "sy8824-reg";
+	rdesc->supply_name = "vin";
+	rdesc->ops = &sy8824_regulator_ops;
+	rdesc->type = REGULATOR_VOLTAGE;
+	rdesc->n_voltages = cfg->vsel_count;
+	rdesc->enable_reg = cfg->enable_reg;
+	rdesc->enable_mask = BUCK_EN;
+	rdesc->min_uV = cfg->vsel_min;
+	rdesc->uV_step = cfg->vsel_step;
+	rdesc->vsel_reg = cfg->vol_reg;
+	rdesc->vsel_mask = cfg->vsel_count - 1;
+	rdesc->owner = THIS_MODULE;
+
+	rdev = devm_regulator_register(di->dev, &di->desc, config);
+	return PTR_ERR_OR_ZERO(rdev);
+}
+
+static const struct regmap_config sy8824_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int sy8824_i2c_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct device_node *np = dev->of_node;
+	struct sy8824_device_info *di;
+	struct regulator_config config = { };
+	struct regmap *regmap;
+	int ret;
+
+	di = devm_kzalloc(dev, sizeof(struct sy8824_device_info), GFP_KERNEL);
+	if (!di)
+		return -ENOMEM;
+
+	di->regulator = of_get_regulator_init_data(dev, np, &di->desc);
+	if (!di->regulator) {
+		dev_err(dev, "Platform data not found!\n");
+		return -EINVAL;
+	}
+
+	di->dev = dev;
+	di->cfg = of_device_get_match_data(dev);
+
+	regmap = devm_regmap_init_i2c(client, &sy8824_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "Failed to allocate regmap!\n");
+		return PTR_ERR(regmap);
+	}
+	i2c_set_clientdata(client, di);
+
+	config.dev = di->dev;
+	config.init_data = di->regulator;
+	config.regmap = regmap;
+	config.driver_data = di;
+	config.of_node = np;
+
+	ret = sy8824_regulator_register(di, &config);
+	if (ret < 0)
+		dev_err(dev, "Failed to register regulator!\n");
+	return ret;
+}
+
+static const struct sy8824_config sy8824c_cfg = {
+	.vol_reg = 0x00,
+	.mode_reg = 0x00,
+	.enable_reg = 0x00,
+	.vsel_min = 762500,
+	.vsel_step = 12500,
+	.vsel_count = 64,
+};
+
+static const struct of_device_id sy8824_dt_ids[] = {
+	{
+		.compatible = "silergy,sy8824c",
+		.data = &sy8824c_cfg
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sy8824_dt_ids);
+
+static const struct i2c_device_id sy8824_id[] = {
+	{ "sy8824", },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, sy8824_id);
+
+static struct i2c_driver sy8824_regulator_driver = {
+	.driver = {
+		.name = "sy8824-regulator",
+		.of_match_table = of_match_ptr(sy8824_dt_ids),
+	},
+	.probe = sy8824_i2c_probe,
+	.id_table = sy8824_id,
+};
+module_i2c_driver(sy8824_regulator_driver);
+
+MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>");
+MODULE_DESCRIPTION("SY8824C regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
2.23.0.rc1

[PATCH 3/8] dt-bindings: sy8824x: Document SY8824E support

[Jisheng Zhang]

SY8824E is an I2C-controlled adjustable voltage regulator made by
Silergy Corp. The only difference between SY8824C and SY8824E is the
vsel_min.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 Documentation/devicetree/bindings/regulator/sy8824x.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/sy8824x.txt b/Documentation/devicetree/bindings/regulator/sy8824x.txt
index ff8d1af04f7b..31fefa3baa71 100644
--- a/Documentation/devicetree/bindings/regulator/sy8824x.txt
+++ b/Documentation/devicetree/bindings/regulator/sy8824x.txt
@@ -1,7 +1,9 @@
-SY8824C Voltage regulator
+SY8824C/SY8824E Voltage regulator
 
 Required properties:
-- compatible: Must be "silergy,sy8824c"
+- compatible: Must be one of the following.
+	"silergy,sy8824c"
+	"silergy,sy8824e"
 - reg: I2C slave address
 
 Any property defined as part of the core regulator binding, defined in
-- 
2.23.0.rc1

[PATCH 4/8] regulator: sy8824x: add SY8824E support

[Jisheng Zhang]

The only difference between SY8824E and SY8824C/D is the vsel_min.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/regulator/Kconfig   |  2 +-
 drivers/regulator/sy8824x.c | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e8093a909e85..b70a0a07795b 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -907,7 +907,7 @@ config REGULATOR_SY8106A
 	  This driver supports SY8106A single output regulator.
 
 config REGULATOR_SY8824X
-	tristate "Silergy SY8824C regulator"
+	tristate "Silergy SY8824C/SY8824E regulator"
 	depends on I2C && (OF || COMPILE_TEST)
 	select REGMAP_I2C
 	help
diff --git a/drivers/regulator/sy8824x.c b/drivers/regulator/sy8824x.c
index 2d8a61ca6643..b1438d94eee2 100644
--- a/drivers/regulator/sy8824x.c
+++ b/drivers/regulator/sy8824x.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * SY8824C regulator driver
+ * SY8824C/SY8824E regulator driver
  *
  * Copyright (C) 2019 Synaptics Incorporated
  *
@@ -162,11 +162,24 @@ static const struct sy8824_config sy8824c_cfg = {
 	.vsel_count = 64,
 };
 
+static const struct sy8824_config sy8824e_cfg = {
+	.vol_reg = 0x00,
+	.mode_reg = 0x00,
+	.enable_reg = 0x00,
+	.vsel_min = 700000,
+	.vsel_step = 12500,
+	.vsel_count = 64,
+};
+
 static const struct of_device_id sy8824_dt_ids[] = {
 	{
 		.compatible = "silergy,sy8824c",
 		.data = &sy8824c_cfg
 	},
+	{
+		.compatible = "silergy,sy8824e",
+		.data = &sy8824e_cfg
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sy8824_dt_ids);
@@ -188,5 +201,5 @@ static struct i2c_driver sy8824_regulator_driver = {
 module_i2c_driver(sy8824_regulator_driver);
 
 MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>");
-MODULE_DESCRIPTION("SY8824C regulator driver");
+MODULE_DESCRIPTION("SY8824C/SY8824E regulator driver");
 MODULE_LICENSE("GPL v2");
-- 
2.23.0.rc1

[PATCH 5/8] dt-bindings: sy8824x: Document SY20276 support

[Jisheng Zhang]

SY20276 is an I2C-controlled adjustable voltage regulator made by
Silergy Corp. The differences between SY8824C and SY20276 are
different vsel_min, vsel_step, vsel_count and regs for mode/enable.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 Documentation/devicetree/bindings/regulator/sy8824x.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/sy8824x.txt b/Documentation/devicetree/bindings/regulator/sy8824x.txt
index 31fefa3baa71..28600541b5de 100644
--- a/Documentation/devicetree/bindings/regulator/sy8824x.txt
+++ b/Documentation/devicetree/bindings/regulator/sy8824x.txt
@@ -1,9 +1,10 @@
-SY8824C/SY8824E Voltage regulator
+SY8824C/SY8824E/SY20276 Voltage regulator
 
 Required properties:
 - compatible: Must be one of the following.
 	"silergy,sy8824c"
 	"silergy,sy8824e"
+	"silergy,sy20276"
 - reg: I2C slave address
 
 Any property defined as part of the core regulator binding, defined in
-- 
2.23.0.rc1

[PATCH 6/8] regulator: sy8824x: add SY20276 support

[Jisheng Zhang]

The differences between SY8824C and SY20276 are different vsel_min,
vsel_step, vsel_count and regs for mode/enable.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/regulator/sy8824x.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/regulator/sy8824x.c b/drivers/regulator/sy8824x.c
index b1438d94eee2..9410c3470870 100644
--- a/drivers/regulator/sy8824x.c
+++ b/drivers/regulator/sy8824x.c
@@ -171,6 +171,15 @@ static const struct sy8824_config sy8824e_cfg = {
 	.vsel_count = 64,
 };
 
+static const struct sy8824_config sy20276_cfg = {
+	.vol_reg = 0x00,
+	.mode_reg = 0x01,
+	.enable_reg = 0x01,
+	.vsel_min = 600000,
+	.vsel_step = 10000,
+	.vsel_count = 128,
+};
+
 static const struct of_device_id sy8824_dt_ids[] = {
 	{
 		.compatible = "silergy,sy8824c",
@@ -180,6 +189,10 @@ static const struct of_device_id sy8824_dt_ids[] = {
 		.compatible = "silergy,sy8824e",
 		.data = &sy8824e_cfg
 	},
+	{
+		.compatible = "silergy,sy20276",
+		.data = &sy20276_cfg
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sy8824_dt_ids);
-- 
2.23.0.rc1

[PATCH 7/8] dt-bindings: sy8824x: Document SY20278 support

[Jisheng Zhang]

SY20276 is an I2C-controlled adjustable voltage regulator made by
Silergy Corp. The differences between SY8824C and SY20278 are
different regs for mode/enable.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 Documentation/devicetree/bindings/regulator/sy8824x.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/regulator/sy8824x.txt b/Documentation/devicetree/bindings/regulator/sy8824x.txt
index 28600541b5de..c5e95850c427 100644
--- a/Documentation/devicetree/bindings/regulator/sy8824x.txt
+++ b/Documentation/devicetree/bindings/regulator/sy8824x.txt
@@ -5,6 +5,7 @@ Required properties:
 	"silergy,sy8824c"
 	"silergy,sy8824e"
 	"silergy,sy20276"
+	"silergy,sy20278"
 - reg: I2C slave address
 
 Any property defined as part of the core regulator binding, defined in
-- 
2.23.0.rc1

[PATCH 8/8] regulator: sy8824x: add SY20278 support

[Jisheng Zhang]

The differences between SY8824C and SY20278 are different regs
for mode/enable.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/regulator/sy8824x.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/regulator/sy8824x.c b/drivers/regulator/sy8824x.c
index 9410c3470870..1a7fa4865491 100644
--- a/drivers/regulator/sy8824x.c
+++ b/drivers/regulator/sy8824x.c
@@ -180,6 +180,15 @@ static const struct sy8824_config sy20276_cfg = {
 	.vsel_count = 128,
 };
 
+static const struct sy8824_config sy20278_cfg = {
+	.vol_reg = 0x00,
+	.mode_reg = 0x01,
+	.enable_reg = 0x01,
+	.vsel_min = 762500,
+	.vsel_step = 12500,
+	.vsel_count = 64,
+};
+
 static const struct of_device_id sy8824_dt_ids[] = {
 	{
 		.compatible = "silergy,sy8824c",
@@ -193,6 +202,10 @@ static const struct of_device_id sy8824_dt_ids[] = {
 		.compatible = "silergy,sy20276",
 		.data = &sy20276_cfg
 	},
+	{
+		.compatible = "silergy,sy20278",
+		.data = &sy20278_cfg
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sy8824_dt_ids);
-- 
2.23.0.rc1

Re: [PATCH 2/8] regulator: add support for SY8824C regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Tue, Aug 27, 2019 at 08:45:33AM +0000, Jisheng Zhang wrote:

This looks mostly good and I'll apply it, a couple of small
things though:

> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SY8824C regulator driver
> + *

Please send a patch which updates the entire comment block to be
C++ style so it looks consistent.

> +#define BUCK_EN		(1 << 7)
> +#define MODE		(1 << 6)
> +

Please also add prefixes to these to namespace them, especially
MODE is likely to collide with something later.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/8] regulator: add support for SY8824C regulator

[Jisheng Zhang]

Hi Mark,

On Tue, 27 Aug 2019 20:44:37 +0100 Mark Brown wrote:

> On Tue, Aug 27, 2019 at 08:45:33AM +0000, Jisheng Zhang wrote:
> 
> This looks mostly good and I'll apply it, a couple of small
> things though:
> 
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SY8824C regulator driver
> > + *  
> 
> Please send a patch which updates the entire comment block to be
> C++ style so it looks consistent.

Do you mean update the following style

A:

// SPDX-License-Identifier: GPL-2.0
/*
 * SY8824C regulator driver
 * ...


as B:

// SPDX-License-Identifier: GPL-2.0
// SY8824C regulator driver
// ...

I'm not sure which style is correct. But I see B is commonly used
in lots .c source files in other dirs, such as kernel/ mm/ etc.

Could you please clarify?

Thanks

> 
> > +#define BUCK_EN		(1 << 7)
> > +#define MODE		(1 << 6)
> > +  
> 
> Please also add prefixes to these to namespace them, especially
> MODE is likely to collide with something later.

good idea. Will do in newer version

Re: [PATCH 2/8] regulator: add support for SY8824C regulator

[Jisheng Zhang]

On Wed, 28 Aug 2019 11:27:05 +0800 Jisheng Zhang wrote:

> Hi Mark,
> 
> On Tue, 27 Aug 2019 20:44:37 +0100 Mark Brown wrote:
> 
> > On Tue, Aug 27, 2019 at 08:45:33AM +0000, Jisheng Zhang wrote:
> > 
> > This looks mostly good and I'll apply it, a couple of small
> > things though:
> >   
> > > @@ -0,0 +1,192 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * SY8824C regulator driver
> > > + *    
> > 
> > Please send a patch which updates the entire comment block to be
> > C++ style so it looks consistent.  
> 
> Do you mean update the following style
> 
> A:
> 
> // SPDX-License-Identifier: GPL-2.0
> /*
>  * SY8824C regulator driver
>  * ...
> 
> 
> as B:
> 
> // SPDX-License-Identifier: GPL-2.0
> // SY8824C regulator driver
> // ...
> 
> I'm not sure which style is correct. But I see B is commonly used

sorry, typo. I mean "I see A is commonly used ..."

> in lots .c source files in other dirs, such as kernel/ mm/ etc.
> 
> Could you please clarify?
> 
> Thanks
> 
> >   
> > > +#define BUCK_EN		(1 << 7)
> > > +#define MODE		(1 << 6)
> > > +    
> > 
> > Please also add prefixes to these to namespace them, especially
> > MODE is likely to collide with something later.  
> 
> good idea. Will do in newer version

Re: [PATCH 2/8] regulator: add support for SY8824C regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On Wed, Aug 28, 2019 at 03:38:29AM +0000, Jisheng Zhang wrote:
> On Tue, 27 Aug 2019 20:44:37 +0100 Mark Brown wrote:

> > 
> > Please send a patch which updates the entire comment block to be
> > C++ style so it looks consistent.

> Do you mean update the following style

> A:
> 
> // SPDX-License-Identifier: GPL-2.0
> /*
>  * SY8824C regulator driver
>  * ...

> as B:

> // SPDX-License-Identifier: GPL-2.0
> // SY8824C regulator driver
> // ...

> I'm not sure which style is correct. But I see B is commonly used
> in lots .c source files in other dirs, such as kernel/ mm/ etc.

Yes, please - update to style B.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]