mbox series

[v5,0/3] Add Samsung s2dos05 pmic support

Message ID 20240617-starqltechn_integration_upstream-v5-0-ea1109029ba5@gmail.com
Headers show
Series Add Samsung s2dos05 pmic support | expand

Message

Dzmitry Sankouski Sept. 26, 2024, 9:47 a.m. UTC 
The S2DOS05 is a companion power management IC for the panel and touchscreen
in smart phones. Provides voltage regulators and
ADC for power/current measurements.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
Changes in v5:
- Split patchset per subsystem
- Rewrite cover letter
- Link to v4: https://lore.kernel.org/r/20240913-starqltechn_integration_upstream-v4-0-2d2efd5c5877@gmail.com

Changes in v4:
- Rewrite max77705, max77705_charger, max77705_fuel_gauge from scratch
- Reorder patches:
  - squash max77705 subdevice bindings in core file because
    no resources there
  - split device tree changes
- Use _ as space for filenames in power/supply like the majority
- Replace gcc-845 freq_tbl frequencies patch with new approach,
  based on automatic m/n/pre_div value generation
- Link to v3: https://lore.kernel.org/r/20240618-starqltechn_integration_upstream-v3-0-e3f6662017ac@gmail.com

Changes in version 3:
- s2dos05 driver converted to MFD

Changes in version 2:
- s2dos05 regulator:
  - hex to decimal in regulator values
  - fix compatible value
  - remove interrupt specific code, because it's
    empty in vendor kernel, and I cannot test it on
    available hardware anyway.

---
Dzmitry Sankouski (3):
      dt-bindings: mfd: add samsung,s2dos05
      mfd: sec-core: add s2dos05 support
      regulator: add s2dos05 regulator support

 Documentation/devicetree/bindings/mfd/samsung,s2dos05.yaml |  99 ++++++++++++++++++++++++++++++++++++++
 MAINTAINERS                                                |   4 +-
 drivers/mfd/sec-core.c                                     |  11 +++++
 drivers/regulator/Kconfig                                  |   8 ++++
 drivers/regulator/Makefile                                 |   1 +
 drivers/regulator/s2dos05-regulator.c                      | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/samsung/core.h                           |   1 +
 include/linux/regulator/s2dos05.h                          |  73 ++++++++++++++++++++++++++++
 8 files changed, 371 insertions(+), 2 deletions(-)
---
base-commit: 92fc9636d1471b7f68bfee70c776f7f77e747b97
change-id: 20240617-starqltechn_integration_upstream-bc86850b2fe3

Best regards,

Comments

Krzysztof Kozlowski Sept. 27, 2024, 8:48 a.m. UTC | #1 
On Thu, Sep 26, 2024 at 12:47:30PM +0300, Dzmitry Sankouski wrote:
> Add samsung,s2dos05 MFD module binding.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 27, 2024, 8:51 a.m. UTC | #2 
On Thu, Sep 26, 2024 at 12:47:32PM +0300, Dzmitry Sankouski wrote:
> +static int s2dos05_pmic_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sec_pmic_dev *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, ret;
> +
> +	s2dos05 = devm_kzalloc(dev, sizeof(*s2dos05), GFP_KERNEL);
> +	if (!s2dos05)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, s2dos05);
> +
> +	rdata = devm_kcalloc(dev, rdev_num, sizeof(*rdata), GFP_KERNEL);
> +	if (!rdata)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < rdev_num; i++)
> +		rdata[i].name = regulators[i].name;
> +
> +	s2dos05->regmap = iodev->regmap_pmic;
> +	s2dos05->dev = dev;
> +	if (!dev->of_node)
> +		dev->of_node = dev->parent->of_node;
> +
> +	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);
> +		}
> +	}
> +
> +	return ret;

ret is uninitialized. Please test your code with smatch and sparse.

Since I expect a new version, I will have a comment on the bindings as well.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 27, 2024, 8:54 a.m. UTC | #3 
On Fri, Sep 27, 2024 at 10:51:08AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Sep 26, 2024 at 12:47:32PM +0300, Dzmitry Sankouski wrote:
> > +static int s2dos05_pmic_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct sec_pmic_dev *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, ret;
> > +
> > +	s2dos05 = devm_kzalloc(dev, sizeof(*s2dos05), GFP_KERNEL);
> > +	if (!s2dos05)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, s2dos05);
> > +
> > +	rdata = devm_kcalloc(dev, rdev_num, sizeof(*rdata), GFP_KERNEL);
> > +	if (!rdata)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < rdev_num; i++)
> > +		rdata[i].name = regulators[i].name;
> > +
> > +	s2dos05->regmap = iodev->regmap_pmic;
> > +	s2dos05->dev = dev;
> > +	if (!dev->of_node)
> > +		dev->of_node = dev->parent->of_node;
> > +
> > +	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);
> > +		}
> > +	}
> > +
> > +	return ret;
> 
> ret is uninitialized. Please test your code with smatch and sparse.
> 
> Since I expect a new version, I will have a comment on the bindings as well.

Ah, no, looks fine. I was thinking you missed interrupts, but aparently
device is quite simple and I coould not find interrupts in downstream
source.

Still please investigate above 'ret' and run smatch and sparse.

Best regards,
Krzysztof
Lee Jones Oct. 9, 2024, 2:29 p.m. UTC | #4 
On Thu, 26 Sep 2024 12:47:29 +0300, Dzmitry Sankouski wrote:
> The S2DOS05 is a companion power management IC for the panel and touchscreen
> in smart phones. Provides voltage regulators and
> ADC for power/current measurements.
> 
> 

Applied, thanks!

[1/3] dt-bindings: mfd: add samsung,s2dos05
      commit: 7bde7326deeaab9ae9345e01b4e321218c4679dd
[2/3] mfd: sec-core: add s2dos05 support
      commit: 176b2e5b80c845a6717122afff7eabcb999cec07

--
Lee Jones [李琼斯]