devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
@ 2024-01-06 13:35 Bhavin Sharma
  2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Bhavin Sharma @ 2024-01-06 13:35 UTC (permalink / raw)
  To: sre
  Cc: Bhavin Sharma, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Hardevsinh Palaniya, linux-pm, devicetree, linux-kernel

Adding support for stc3117 fuel gauge driver with
minimal functionality to read voltage level currently

Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
---
 drivers/power/supply/Kconfig              |   7 ++
 drivers/power/supply/Makefile             |   1 +
 drivers/power/supply/stc3117_fuel_gauge.c | 136 ++++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index f21cb0581..02df55b0d 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -875,6 +875,13 @@ config FUEL_GAUGE_SC27XX
 	  Say Y here to enable support for fuel gauge with SC27XX
 	  PMIC chips.
 
+config FUEL_GAUGE_STC3117
+	tristate "STMicroelectronics STC3117 fuel gauge driver"
+	depends on I2C
+	help
+	  Say Y here to enable support for fuel gauge with STC3117
+	  PMIC chips.
+
 config CHARGER_UCS1002
 	tristate "Microchip UCS1002 USB Port Power Controller"
 	depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 58b567278..f8ed3e5a8 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
 obj-$(CONFIG_CHARGER_CROS_PCHG)	+= cros_peripheral_charger.o
 obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
 obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
+obj-$(CONFIG_FUEL_GAUGE_STC3117)	+= stc3117_fuel_gauge.o
 obj-$(CONFIG_CHARGER_UCS1002)	+= ucs1002_power.o
 obj-$(CONFIG_CHARGER_BD99954)	+= bd99954-charger.o
 obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
diff --git a/drivers/power/supply/stc3117_fuel_gauge.c b/drivers/power/supply/stc3117_fuel_gauge.c
new file mode 100644
index 000000000..bf87bc1b9
--- /dev/null
+++ b/drivers/power/supply/stc3117_fuel_gauge.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
+ *
+ * Copyright (c) 2024 Silicon Signals Pvt Ltd.
+ * Author:      Bhavin Sharma <bhavin.sharma@siliconsignals.io>
+ *              Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.com>
+ */
+
+
+#include <linux/i2c.h>
+#include <linux/i2c-dev.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/power_supply.h>
+
+#define VOLTAGE_REG_ADDR	0x08
+#define VOLTAGE_REG_ADDR_SIZE	1		//in bytes
+#define VOLTAGE_DATA_SIZE	2		//in bytes
+#define LSB_VALUE		2200		//in micro-volts
+
+
+static int stc3117_probe(struct i2c_client *client);
+static void stc3117_dev_remove(struct i2c_client *client);
+
+static int stc3117_get_property(struct power_supply *psy,
+	enum power_supply_property psp, union power_supply_propval *val);
+static int stc3117_get_batt_volt(const struct i2c_client *client);
+
+const struct i2c_client *tmp_client;
+struct power_supply *stc_sply;
+
+static const struct of_device_id stc3117_of_match[] = {
+	{ .compatible = "st,stc3117-fgu" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, stc3117_of_match);
+
+static const struct i2c_device_id stc3117_id[] = {
+	{"stc3117", 0},
+	{},
+};
+
+
+MODULE_DEVICE_TABLE(i2c, stc3117_id);
+
+struct i2c_driver stc3117_i2c_driver = {
+	.driver = {
+		.name = "stc3117_i2c_driver",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(stc3117_of_match),
+	},
+	.probe = stc3117_probe,
+	.id_table = stc3117_id,
+	.remove = stc3117_dev_remove,
+};
+
+
+static enum power_supply_property stc3117_battery_props[] = {
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+};
+
+static const struct power_supply_desc stc3117_battery_desc = {
+	.name = "stc3117-battery",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.get_property = stc3117_get_property,
+	.properties = stc3117_battery_props,
+	.num_properties = ARRAY_SIZE(stc3117_battery_props),
+};
+
+static int stc3117_get_property(struct power_supply *psy,
+	enum power_supply_property psp, union power_supply_propval *val)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = stc3117_get_batt_volt(tmp_client);
+	break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
+static int stc3117_get_batt_volt(const struct i2c_client *stc_client)
+{
+	int ret, volt = 0;
+	char i2c_tx = VOLTAGE_REG_ADDR, i2c_rx[2] = {0};
+
+	ret = i2c_master_send(stc_client, &i2c_tx, VOLTAGE_REG_ADDR_SIZE);
+	if (ret > 0) {
+
+		ret = i2c_master_recv(stc_client, i2c_rx, VOLTAGE_DATA_SIZE);
+		if (ret > 0) {
+
+			volt = (i2c_rx[1] << 8) + i2c_rx[0];
+			volt *= LSB_VALUE;
+
+			return volt;
+		}
+	}
+
+	return ret;
+}
+
+static int stc3117_probe(struct i2c_client *client)
+{
+	struct power_supply_config psy_cfg = {};
+	struct device *dev;
+
+	dev = &client->dev;
+
+	psy_cfg.of_node = dev->of_node;
+
+	tmp_client = client;
+
+	stc_sply = power_supply_register(dev, &stc3117_battery_desc, &psy_cfg);
+	if (IS_ERR(stc_sply))
+		pr_err("failed to register battery\n");
+
+	return 0;
+}
+
+static void stc3117_dev_remove(struct i2c_client *client)
+{
+	power_supply_unregister(stc_sply);
+}
+
+module_i2c_driver(stc3117_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bhavin Sharma <bhavin.sharma@siliconsignals.io>");
+MODULE_AUTHOR("Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>");
+MODULE_DESCRIPTION("STC3117 Fuel Gauge Driver");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format
  2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
@ 2024-01-06 13:35 ` Bhavin Sharma
  2024-01-06 14:36   ` Rob Herring
  2024-01-06 16:48   ` Krzysztof Kozlowski
  2024-01-06 16:52 ` [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 8+ messages in thread
From: Bhavin Sharma @ 2024-01-06 13:35 UTC (permalink / raw)
  To: sre
  Cc: Bhavin Sharma, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Hardevsinh Palaniya, linux-pm, devicetree, linux-kernel

Convert the binding to DT schema format.

Changes in V2 resolved below errors:
	1. string value is redundantly quoted with any quotes (quoted-strings)
	2. found character '\t' that cannot start any token

Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
---
 .../bindings/power/supply/stc3117-fg.yaml     | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml b/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
new file mode 100644
index 000000000..d6607a5ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/stc3117-fg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STC3117 Fuel Gauge Unit Power Supply
+
+maintainers:
+  - Bhavin Sharma <bhavin.sharma@siliconsignals.io>
+  - Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
+
+allOf:
+  - $ref: power-supply.yaml#
+
+properties:
+  compatible:
+    enum:
+      - st,stc3117-fgu
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    &i2c6 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      st3117: stc3117@70 {
+        compatible = "st,stc3117-fgu";
+        reg = <0x70>;
+        status = "okay";
+      };
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format
  2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
@ 2024-01-06 14:36   ` Rob Herring
  2024-01-06 16:48   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Rob Herring @ 2024-01-06 14:36 UTC (permalink / raw)
  To: Bhavin Sharma
  Cc: Krzysztof Kozlowski, Rob Herring, sre, Hardevsinh Palaniya,
	linux-kernel, Conor Dooley, linux-pm, devicetree


On Sat, 06 Jan 2024 19:05:44 +0530, Bhavin Sharma wrote:
> Convert the binding to DT schema format.
> 
> Changes in V2 resolved below errors:
> 	1. string value is redundantly quoted with any quotes (quoted-strings)
> 	2. found character '\t' that cannot start any token
> 
> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> ---
>  .../bindings/power/supply/stc3117-fg.yaml     | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/power/supply/stc3117-fg.example.dts:18.9-14 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/power/supply/stc3117-fg.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240106133546.936261-2-bhavin.sharma@siliconsignals.io

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format
  2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
  2024-01-06 14:36   ` Rob Herring
@ 2024-01-06 16:48   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-06 16:48 UTC (permalink / raw)
  To: Bhavin Sharma, sre
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Hardevsinh Palaniya, linux-pm, devicetree, linux-kernel

On 06/01/2024 14:35, Bhavin Sharma wrote:
> Convert the binding to DT schema format.

Subject: It's totally wrong. There is no conversion. If you claim
otherwise, please point me to removal of any file in this patch.

> 
> Changes in V2 resolved below errors:

Changelog goes under ---

> 	1. string value is redundantly quoted with any quotes (quoted-strings)
> 	2. found character '\t' that cannot start any token

Heh, you still did not test it.


> 
> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> ---
>  .../bindings/power/supply/stc3117-fg.yaml     | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml b/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
> new file mode 100644
> index 000000000..d6607a5ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/stc3117-fg.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/stc3117-fg.yaml#

Namke matching compatible.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics STC3117 Fuel Gauge Unit Power Supply
> +
> +maintainers:
> +  - Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> +  - Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
> +
> +allOf:
> +  - $ref: power-supply.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - st,stc3117-fgu

Can stc3117 be anything else? IOW, why "fgu" is needed?

> +
> +  reg:
> +    maxItems: 1

No properties? No resources?

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

unevaluatedProperties instead

> +
> +examples:
> +  - |
> +    &i2c6 {

Just i2c {

> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      st3117: stc3117@70 {

Drop label.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +        compatible = "st,stc3117-fgu";
> +        reg = <0x70>;
> +        status = "okay";

Drop status. Add missing properties for this to be complete.

> +      };
> +    };

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
  2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
  2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
@ 2024-01-06 16:52 ` Krzysztof Kozlowski
  2024-01-06 16:53 ` Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-06 16:52 UTC (permalink / raw)
  To: Bhavin Sharma, sre
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Hardevsinh Palaniya, linux-pm, devicetree, linux-kernel

On 06/01/2024 14:35, Bhavin Sharma wrote:
> Adding support for stc3117 fuel gauge driver with
> minimal functionality to read voltage level currently
> 
> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> ---
>  drivers/power/supply/Kconfig              |   7 ++
>  drivers/power/supply/Makefile             |   1 +
>  drivers/power/supply/stc3117_fuel_gauge.c | 136 ++++++++++++++++++++++
>  3 files changed, 144 insertions(+)
>  create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index f21cb0581..02df55b0d 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -875,6 +875,13 @@ config FUEL_GAUGE_SC27XX
>  	  Say Y here to enable support for fuel gauge with SC27XX
>  	  PMIC chips.
>  
> +config FUEL_GAUGE_STC3117
> +	tristate "STMicroelectronics STC3117 fuel gauge driver"
> +	depends on I2C
> +	help
> +	  Say Y here to enable support for fuel gauge with STC3117
> +	  PMIC chips.
> +
>  config CHARGER_UCS1002
>  	tristate "Microchip UCS1002 USB Port Power Controller"
>  	depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 58b567278..f8ed3e5a8 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -104,6 +104,7 @@ obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
>  obj-$(CONFIG_CHARGER_CROS_PCHG)	+= cros_peripheral_charger.o
>  obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
>  obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
> +obj-$(CONFIG_FUEL_GAUGE_STC3117)	+= stc3117_fuel_gauge.o
>  obj-$(CONFIG_CHARGER_UCS1002)	+= ucs1002_power.o
>  obj-$(CONFIG_CHARGER_BD99954)	+= bd99954-charger.o
>  obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
> diff --git a/drivers/power/supply/stc3117_fuel_gauge.c b/drivers/power/supply/stc3117_fuel_gauge.c
> new file mode 100644
> index 000000000..bf87bc1b9
> --- /dev/null
> +++ b/drivers/power/supply/stc3117_fuel_gauge.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
> + *
> + * Copyright (c) 2024 Silicon Signals Pvt Ltd.
> + * Author:      Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> + *              Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.com>
> + */
> +
> +

Just one blank line

> +#include <linux/i2c.h>
> +#include <linux/i2c-dev.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +#include <linux/power_supply.h>
> +
> +#define VOLTAGE_REG_ADDR	0x08
> +#define VOLTAGE_REG_ADDR_SIZE	1		//in bytes
> +#define VOLTAGE_DATA_SIZE	2		//in bytes
> +#define LSB_VALUE		2200		//in micro-volts

LSB of what? This is very poor naming.


Missing spaces after //

> +
> +

One blank line

> +static int stc3117_probe(struct i2c_client *client);
> +static void stc3117_dev_remove(struct i2c_client *client);
> +
> +static int stc3117_get_property(struct power_supply *psy,
> +	enum power_supply_property psp, union power_supply_propval *val);
> +static int stc3117_get_batt_volt(const struct i2c_client *client);

Drop all these.

> +
> +const struct i2c_client *tmp_client;

Drop

> +struct power_supply *stc_sply;

Drop

> +
> +static const struct of_device_id stc3117_of_match[] = {

This goes next to driver structure.

> +	{ .compatible = "st,stc3117-fgu" },
> +	{},
> +};
> +

No blank line

> +MODULE_DEVICE_TABLE(of, stc3117_of_match);
> +
> +static const struct i2c_device_id stc3117_id[] = {

The same.

> +	{"stc3117", 0},
> +	{},
> +};
> +
> +

No blank lines

> +MODULE_DEVICE_TABLE(i2c, stc3117_id);
> +
> +struct i2c_driver stc3117_i2c_driver = {

Where did you place it? Open existing drivers and see how it is done.

> +	.driver = {
> +		.name = "stc3117_i2c_driver",
> +		.owner = THIS_MODULE,

Drop, ok, this is some ancient vendor driver which you just send
upstream. Sorry, no. You must clean it from all that ancient cruft.

You must run smatch, sparse and coccinelle/coccicheck.

> +		.of_match_table = of_match_ptr(stc3117_of_match),

Drop of_match_ptr

> +	},
> +	.probe = stc3117_probe,
> +	.id_table = stc3117_id,
> +	.remove = stc3117_dev_remove,
> +};
> +
> +
> +static enum power_supply_property stc3117_battery_props[] = {
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +};
> +
> +static const struct power_supply_desc stc3117_battery_desc = {
> +	.name = "stc3117-battery",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.get_property = stc3117_get_property,
> +	.properties = stc3117_battery_props,
> +	.num_properties = ARRAY_SIZE(stc3117_battery_props),
> +};
> +
> +static int stc3117_get_property(struct power_supply *psy,
> +	enum power_supply_property psp, union power_supply_propval *val)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		val->intval = stc3117_get_batt_volt(tmp_client);
> +	break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +

What's with this double blank lines?

> +static int stc3117_get_batt_volt(const struct i2c_client *stc_client)
> +{
> +	int ret, volt = 0;
> +	char i2c_tx = VOLTAGE_REG_ADDR, i2c_rx[2] = {0};
> +
> +	ret = i2c_master_send(stc_client, &i2c_tx, VOLTAGE_REG_ADDR_SIZE);
> +	if (ret > 0) {
> +
> +		ret = i2c_master_recv(stc_client, i2c_rx, VOLTAGE_DATA_SIZE);
> +		if (ret > 0) {
> +
> +			volt = (i2c_rx[1] << 8) + i2c_rx[0];
> +			volt *= LSB_VALUE;
> +
> +			return volt;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int stc3117_probe(struct i2c_client *client)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct device *dev;
> +
> +	dev = &client->dev;
> +
> +	psy_cfg.of_node = dev->of_node;
> +
> +	tmp_client = client;
> +
> +	stc_sply = power_supply_register(dev, &stc3117_battery_desc, &psy_cfg);

devm

> +	if (IS_ERR(stc_sply))
> +		pr_err("failed to register battery\n");

dev_err

> +
> +	return 0;
> +}
> +
> +static void stc3117_dev_remove(struct i2c_client *client)
> +{
> +	power_supply_unregister(stc_sply);
> +}
> +
> +module_i2c_driver(stc3117_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Bhavin Sharma <bhavin.sharma@siliconsignals.io>");
> +MODULE_AUTHOR("Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>");
> +MODULE_DESCRIPTION("STC3117 Fuel Gauge Driver");

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
  2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
  2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
  2024-01-06 16:52 ` [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Krzysztof Kozlowski
@ 2024-01-06 16:53 ` Krzysztof Kozlowski
  2024-01-07 22:11 ` kernel test robot
  2024-01-11 15:30 ` kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-06 16:53 UTC (permalink / raw)
  To: Bhavin Sharma, sre
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Hardevsinh Palaniya, linux-pm, devicetree, linux-kernel

On 06/01/2024 14:35, Bhavin Sharma wrote:
> Adding support for stc3117 fuel gauge driver with
> minimal functionality to read voltage level currently
> 
> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> ---
>  drivers/power/supply/Kconfig              |   7 ++
>  drivers/power/supply/Makefile             |   1 +
>  drivers/power/supply/stc3117_fuel_gauge.c | 136 ++++++++++++++++++++++
>  3 files changed, 144 insertions(+)
>  create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c
> 

I see now that you got review for your v1. You must implement it fully.
Second, don't send patchsets more often than one per day to allow people
to actually review your code. Especially if you do not test it (neither
v1 nor v2!).

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
  2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
                   ` (2 preceding siblings ...)
  2024-01-06 16:53 ` Krzysztof Kozlowski
@ 2024-01-07 22:11 ` kernel test robot
  2024-01-11 15:30 ` kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-01-07 22:11 UTC (permalink / raw)
  To: Bhavin Sharma, sre
  Cc: oe-kbuild-all, Bhavin Sharma, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hardevsinh Palaniya, linux-pm, devicetree,
	linux-kernel

Hi Bhavin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on robh/for-next linus/master v6.7-rc8 next-20240105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bhavin-Sharma/dt-bindings-power-supply-stc3117-Convert-to-DT-schema-format/20240106-213744
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240106133546.936261-1-bhavin.sharma%40siliconsignals.io
patch subject: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
config: alpha-randconfig-r112-20240108 (https://download.01.org/0day-ci/archive/20240108/202401080530.0hMWnrIg-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240108/202401080530.0hMWnrIg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401080530.0hMWnrIg-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/power/supply/stc3117_fuel_gauge.c:34:34: warning: 'stc3117_of_match' defined but not used [-Wunused-const-variable=]
      34 | static const struct of_device_id stc3117_of_match[] = {
         |                                  ^~~~~~~~~~~~~~~~


vim +/stc3117_of_match +34 drivers/power/supply/stc3117_fuel_gauge.c

    33	
  > 34	static const struct of_device_id stc3117_of_match[] = {
    35		{ .compatible = "st,stc3117-fgu" },
    36		{},
    37	};
    38	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
  2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
                   ` (3 preceding siblings ...)
  2024-01-07 22:11 ` kernel test robot
@ 2024-01-11 15:30 ` kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-01-11 15:30 UTC (permalink / raw)
  To: Bhavin Sharma, sre
  Cc: oe-kbuild-all, Bhavin Sharma, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hardevsinh Palaniya, linux-pm, devicetree,
	linux-kernel

Hi Bhavin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on robh/for-next linus/master v6.7 next-20240111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bhavin-Sharma/dt-bindings-power-supply-stc3117-Convert-to-DT-schema-format/20240106-213744
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240106133546.936261-1-bhavin.sharma%40siliconsignals.io
patch subject: [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver
config: x86_64-randconfig-r131-20240111 (https://download.01.org/0day-ci/archive/20240111/202401112330.LVTGnI1p-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240111/202401112330.LVTGnI1p-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401112330.LVTGnI1p-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/power/supply/stc3117_fuel_gauge.c:31:25: sparse: sparse: symbol 'tmp_client' was not declared. Should it be static?
>> drivers/power/supply/stc3117_fuel_gauge.c:32:21: sparse: sparse: symbol 'stc_sply' was not declared. Should it be static?
>> drivers/power/supply/stc3117_fuel_gauge.c:49:19: sparse: sparse: symbol 'stc3117_i2c_driver' was not declared. Should it be static?

vim +/tmp_client +31 drivers/power/supply/stc3117_fuel_gauge.c

    30	
  > 31	const struct i2c_client *tmp_client;
  > 32	struct power_supply *stc_sply;
    33	
    34	static const struct of_device_id stc3117_of_match[] = {
    35		{ .compatible = "st,stc3117-fgu" },
    36		{},
    37	};
    38	
    39	MODULE_DEVICE_TABLE(of, stc3117_of_match);
    40	
    41	static const struct i2c_device_id stc3117_id[] = {
    42		{"stc3117", 0},
    43		{},
    44	};
    45	
    46	
    47	MODULE_DEVICE_TABLE(i2c, stc3117_id);
    48	
  > 49	struct i2c_driver stc3117_i2c_driver = {
    50		.driver = {
    51			.name = "stc3117_i2c_driver",
    52			.owner = THIS_MODULE,
    53			.of_match_table = of_match_ptr(stc3117_of_match),
    54		},
    55		.probe = stc3117_probe,
    56		.id_table = stc3117_id,
    57		.remove = stc3117_dev_remove,
    58	};
    59	
    60	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-01-11 15:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-06 13:35 [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Bhavin Sharma
2024-01-06 13:35 ` [PATCH v2 2/2] dt-bindings: power: supply: stc3117: Convert to DT schema format Bhavin Sharma
2024-01-06 14:36   ` Rob Herring
2024-01-06 16:48   ` Krzysztof Kozlowski
2024-01-06 16:52 ` [PATCH v2 1/2] power: supply: Add STC3117 fuel gauge unit driver Krzysztof Kozlowski
2024-01-06 16:53 ` Krzysztof Kozlowski
2024-01-07 22:11 ` kernel test robot
2024-01-11 15:30 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).