[PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support

Dzmitry Sankouski posted 10 patches 11 months, 2 weeks ago
There is a newer version of this series
[PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Dzmitry Sankouski 11 months, 2 weeks ago
Add MAX77705 support - fuel gauge and hwmon devices.
Hwmon provides charger input and system bus measurements.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
Changes in v13:
- remove compatible from cells
- change mfd compatible to match max77705 fuel gauge node
---
 drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 6eda79533208..22159913bea0 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -83,11 +83,22 @@ static const struct simple_mfd_data maxim_max5970 = {
 	.mfd_cell_size = ARRAY_SIZE(max5970_cells),
 };
 
+static const struct mfd_cell max77705_sensor_cells[] = {
+	{ .name = "max77705-battery" },
+	{ .name = "max77705-hwmon", },
+};
+
+static const struct simple_mfd_data maxim_mon_max77705 = {
+	.mfd_cell = max77705_sensor_cells,
+	.mfd_cell_size = ARRAY_SIZE(max77705_sensor_cells),
+};
+
 static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "kontron,sl28cpld" },
 	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
 	{ .compatible = "maxim,max5970", .data = &maxim_max5970},
 	{ .compatible = "maxim,max5978", .data = &maxim_max5970},
+	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

-- 
2.39.5
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Lee Jones 11 months, 2 weeks ago
On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:

> Add MAX77705 support - fuel gauge and hwmon devices.
> Hwmon provides charger input and system bus measurements.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
> Changes in v13:
> - remove compatible from cells
> - change mfd compatible to match max77705 fuel gauge node
> ---
>  drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index 6eda79533208..22159913bea0 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -83,11 +83,22 @@ static const struct simple_mfd_data maxim_max5970 = {
>  	.mfd_cell_size = ARRAY_SIZE(max5970_cells),
>  };
>  
> +static const struct mfd_cell max77705_sensor_cells[] = {
> +	{ .name = "max77705-battery" },
> +	{ .name = "max77705-hwmon", },
> +};
> +
> +static const struct simple_mfd_data maxim_mon_max77705 = {
> +	.mfd_cell = max77705_sensor_cells,
> +	.mfd_cell_size = ARRAY_SIZE(max77705_sensor_cells),
> +};
> +
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
>  	{ .compatible = "kontron,sl28cpld" },
>  	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
>  	{ .compatible = "maxim,max5970", .data = &maxim_max5970},
>  	{ .compatible = "maxim,max5978", .data = &maxim_max5970},
> +	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},

Drop the battery part from the MFD (group) name please.

>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> 
> -- 
> 2.39.5
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Dzmitry Sankouski 11 months, 1 week ago
чт, 9 янв. 2025 г. в 15:02, Lee Jones <lee@kernel.org>:
>
> On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
>
> > Add MAX77705 support - fuel gauge and hwmon devices.
> > Hwmon provides charger input and system bus measurements.
> >
> > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
(...)
> >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> >       { .compatible = "kontron,sl28cpld" },
> >       { .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> >       { .compatible = "maxim,max5970", .data = &maxim_max5970},
> >       { .compatible = "maxim,max5978", .data = &maxim_max5970},
> > +     { .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
>
> Drop the battery part from the MFD (group) name please.
>

It will then conflict with MAX77705 mfd driver compatible.

-- 
Best regards and thanks for review,
Dzmitry
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Lee Jones 11 months ago
On Wed, 15 Jan 2025, Dzmitry Sankouski wrote:

> чт, 9 янв. 2025 г. в 15:02, Lee Jones <lee@kernel.org>:
> >
> > On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
> >
> > > Add MAX77705 support - fuel gauge and hwmon devices.
> > > Hwmon provides charger input and system bus measurements.
> > >
> > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> (...)
> > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > >       { .compatible = "kontron,sl28cpld" },
> > >       { .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> > >       { .compatible = "maxim,max5970", .data = &maxim_max5970},
> > >       { .compatible = "maxim,max5978", .data = &maxim_max5970},
> > > +     { .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
> >
> > Drop the battery part from the MFD (group) name please.
> >
> 
> It will then conflict with MAX77705 mfd driver compatible.

Where is that used?

-- 
Lee Jones [李琼斯]
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Dzmitry Sankouski 11 months ago
вт, 21 янв. 2025 г. в 13:15, Lee Jones <lee@kernel.org>:
>
> On Wed, 15 Jan 2025, Dzmitry Sankouski wrote:
>
> > чт, 9 янв. 2025 г. в 15:02, Lee Jones <lee@kernel.org>:
> > >
> > > On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
> > >
> > > > Add MAX77705 support - fuel gauge and hwmon devices.
> > > > Hwmon provides charger input and system bus measurements.
> > > >
> > > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > (...)
> > > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > > >       { .compatible = "kontron,sl28cpld" },
> > > >       { .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> > > >       { .compatible = "maxim,max5970", .data = &maxim_max5970},
> > > >       { .compatible = "maxim,max5978", .data = &maxim_max5970},
> > > > +     { .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
> > >
> > > Drop the battery part from the MFD (group) name please.
> > >
> >
> > It will then conflict with MAX77705 mfd driver compatible.
>
> Where is that used?

In MAX77705 MFD patch:
https://patchwork.kernel.org/project/linux-pm/patch/20250117-starqltechn_integration_upstream-v16-5-11afa877276c@gmail.com/

-- 
Best regards and thanks for review,
Dzmitry
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Lee Jones 11 months, 2 weeks ago
On Thu, 09 Jan 2025, Lee Jones wrote:

> On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
> 
> > Add MAX77705 support - fuel gauge and hwmon devices.
> > Hwmon provides charger input and system bus measurements.
> > 
> > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > ---
> > Changes in v13:
> > - remove compatible from cells
> > - change mfd compatible to match max77705 fuel gauge node
> > ---
> >  drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > index 6eda79533208..22159913bea0 100644
> > --- a/drivers/mfd/simple-mfd-i2c.c
> > +++ b/drivers/mfd/simple-mfd-i2c.c
> > @@ -83,11 +83,22 @@ static const struct simple_mfd_data maxim_max5970 = {
> >  	.mfd_cell_size = ARRAY_SIZE(max5970_cells),
> >  };
> >  
> > +static const struct mfd_cell max77705_sensor_cells[] = {
> > +	{ .name = "max77705-battery" },
> > +	{ .name = "max77705-hwmon", },
> > +};

Why not register these from the proper MFD driver?

> > +static const struct simple_mfd_data maxim_mon_max77705 = {
> > +	.mfd_cell = max77705_sensor_cells,
> > +	.mfd_cell_size = ARRAY_SIZE(max77705_sensor_cells),
> > +};
> > +
> >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> >  	{ .compatible = "kontron,sl28cpld" },
> >  	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> >  	{ .compatible = "maxim,max5970", .data = &maxim_max5970},
> >  	{ .compatible = "maxim,max5978", .data = &maxim_max5970},
> > +	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
> 
> Drop the battery part from the MFD (group) name please.
> 
> >  	{}
> >  };
> >  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

-- 
Lee Jones [李琼斯]
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Dzmitry Sankouski 11 months, 2 weeks ago
чт, 9 янв. 2025 г. в 15:03, Lee Jones <lee@kernel.org>:
>
> On Thu, 09 Jan 2025, Lee Jones wrote:
>
> > On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
> >
> > > Add MAX77705 support - fuel gauge and hwmon devices.
> > > Hwmon provides charger input and system bus measurements.
> > >
> > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > > ---
> > > Changes in v13:
> > > - remove compatible from cells
> > > - change mfd compatible to match max77705 fuel gauge node
> > > ---
> > >  drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > index 6eda79533208..22159913bea0 100644
> > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > @@ -83,11 +83,22 @@ static const struct simple_mfd_data maxim_max5970 = {
> > >     .mfd_cell_size = ARRAY_SIZE(max5970_cells),
> > >  };
> > >
> > > +static const struct mfd_cell max77705_sensor_cells[] = {
> > > +   { .name = "max77705-battery" },
> > > +   { .name = "max77705-hwmon", },
> > > +};
>
> Why not register these from the proper MFD driver?
>

Because the fuel gauge address is different from the max77705 mfd device.

-- 
Best regards and thanks for review,
Dzmitry
Re: [PATCH v14 07/10] mfd: simple-mfd-i2c: Add MAX77705 support
Posted by Dzmitry Sankouski 11 months, 1 week ago
чт, 9 янв. 2025 г. в 15:53, Dzmitry Sankouski <dsankouski@gmail.com>:
>
> чт, 9 янв. 2025 г. в 15:03, Lee Jones <lee@kernel.org>:
> >
> > On Thu, 09 Jan 2025, Lee Jones wrote:
> >
> > > On Wed, 08 Jan 2025, Dzmitry Sankouski wrote:
> > >
> > > > Add MAX77705 support - fuel gauge and hwmon devices.
> > > > Hwmon provides charger input and system bus measurements.
> > > >
> > > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > > > ---
> > > > Changes in v13:
> > > > - remove compatible from cells
> > > > - change mfd compatible to match max77705 fuel gauge node
> > > > ---
> > > >  drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > > index 6eda79533208..22159913bea0 100644
> > > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > > @@ -83,11 +83,22 @@ static const struct simple_mfd_data maxim_max5970 = {
> > > >     .mfd_cell_size = ARRAY_SIZE(max5970_cells),
> > > >  };
> > > >
> > > > +static const struct mfd_cell max77705_sensor_cells[] = {
> > > > +   { .name = "max77705-battery" },
> > > > +   { .name = "max77705-hwmon", },
> > > > +};
> >
> > Why not register these from the proper MFD driver?
> >
>
> Because the fuel gauge address is different from the max77705 mfd device.

In more details:

we had a discussion with Krzysztof about fuel gauge device
[1], [2], [3] and agreed that it should be modeled as a separate device,
because it has no common resources with max77705 device, and has separate
address. This means its node are out of MAX77705 mfd node, forming its own
MFD with shared i2c bus.

https://lore.kernel.org/lkml/55f32164-f504-4409-8ce2-6462b833da89@kernel.org/
https://patchwork.kernel.org/project/linux-input/patch/20241202-starqltechn_integration_upstream-v9-3-a1adc3bae2b8@gmail.com/
https://patches.linaro.org/project/linux-leds/patch/20241217-starqltechn_integration_upstream-v12-2-ed840944f948@gmail.com/#951752

-- 
Best regards and thanks for review,
Dzmitry