mbox series

[v5,0/8] Add support for mp2733 battery charger

Message ID 20221029093000.45451-1-sravanhome@gmail.com
Headers show
Series Add support for mp2733 battery charger | expand

Message

Saravanan Sekar Oct. 29, 2022, 9:29 a.m. UTC
changes in v5:
  - fixed commit message on v5-0002 and v5-0004

changes in v4:
  - fixed attributes groups review comments in v3
  - added new bug fix patches v4-0007 and v4-0008 

changes in v3:
  - fixed dt_binding_check error
  - fixed spelling usb->USB

changes in v2:
  - fixed spelling
  - revert back probe to probe_new in mfd driver

I do not see a cover letter, but FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for all patches except DT binding
Note, some of the comments regarding spelling were given, I believe
you are going to address them in v3.


add support for mp2733 Battery charger control driver for Monolithic
Power System's MP2733 chipset 

Saravanan Sekar (8):
  iio: adc: mp2629: fix wrong comparison of channel
  mfd: mp2629: fix failed to get iio channel by device name
  iio: adc: mp2629: fix potential array out of bound access
  power: supply: fix wrong interpretation of register value
  mfd: mp2629: Add support for mps mp2733 battery charger
  iio: adc: mp2629: restrict input voltage mask for mp2629
  power: supply: Add support for mp2733 battery charger
  power: supply: mp2629: Add USB fast charge settings

 .../ABI/testing/sysfs-class-power-mp2629      |  16 ++
 drivers/iio/adc/mp2629_adc.c                  |   8 +-
 drivers/mfd/mp2629.c                          |   7 +-
 drivers/power/supply/mp2629_charger.c         | 229 +++++++++++++++---
 include/linux/mfd/mp2629.h                    |   6 +
 5 files changed, 228 insertions(+), 38 deletions(-)

Comments

Jonathan Cameron Oct. 29, 2022, 12:28 p.m. UTC | #1
On Sat, 29 Oct 2022 11:29:53 +0200
Saravanan Sekar <sravanhome@gmail.com> wrote:

> Input voltage channel enum is compared against iio address instead
> of the channel.
> 
> Fixes: 7abd9fb64682 ("iio: adc: mp2629: Add support for mp2629 ADC driver")
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,
Jonathan

> ---
>  drivers/iio/adc/mp2629_adc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
> index 30a31f185d08..f7af9af1665d 100644
> --- a/drivers/iio/adc/mp2629_adc.c
> +++ b/drivers/iio/adc/mp2629_adc.c
> @@ -74,7 +74,7 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		if (chan->address == MP2629_INPUT_VOLT)
> +		if (chan->channel == MP2629_INPUT_VOLT)
>  			rval &= GENMASK(6, 0);
>  		*val = rval;
>  		return IIO_VAL_INT;
Jonathan Cameron Oct. 29, 2022, 12:30 p.m. UTC | #2
On Sat, 29 Oct 2022 11:29:55 +0200
Saravanan Sekar <sravanhome@gmail.com> wrote:

> Add sentinel at end of maps to avoid potential array out of
> bound access in iio core.
> 
> Fixes: 7abd9fb6468 ("iio: adc: mp2629: Add support for mp2629 ADC driver")
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/mp2629_adc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
> index f7af9af1665d..88e947f300cf 100644
> --- a/drivers/iio/adc/mp2629_adc.c
> +++ b/drivers/iio/adc/mp2629_adc.c
> @@ -57,7 +57,8 @@ static struct iio_map mp2629_adc_maps[] = {
>  	MP2629_MAP(SYSTEM_VOLT, "system-volt"),
>  	MP2629_MAP(INPUT_VOLT, "input-volt"),
>  	MP2629_MAP(BATT_CURRENT, "batt-current"),
> -	MP2629_MAP(INPUT_CURRENT, "input-current")
> +	MP2629_MAP(INPUT_CURRENT, "input-current"),
> +	{ }
>  };
>  
>  static int mp2629_read_raw(struct iio_dev *indio_dev,
Jonathan Cameron Oct. 29, 2022, 12:35 p.m. UTC | #3
On Sat, 29 Oct 2022 11:29:58 +0200
Saravanan Sekar <sravanhome@gmail.com> wrote:

> Add support for mp2733 which is updated version of mp2629
> with a higher range of input voltage.
> 
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Hmm. this is going to slow things down wrt to this series going in.
We want the associated fix to go in during this cycle, and this
is dependant on a change going via MFD.  Ah well.

Generally I prefer to avoid using chip IDs for this sort of check
because they don't generalize as well as an actual flag for this
'feature' stored in a chip_info type structure.

However, we can tidy that up if it becomes relevant as more parts
are added to the driver.

On basis this might end up going via MFD,

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/iio/adc/mp2629_adc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
> index 88e947f300cf..18290e176e1e 100644
> --- a/drivers/iio/adc/mp2629_adc.c
> +++ b/drivers/iio/adc/mp2629_adc.c
> @@ -66,6 +66,7 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
>  			int *val, int *val2, long mask)
>  {
>  	struct mp2629_adc *info = iio_priv(indio_dev);
> +	struct mp2629_data *ddata = dev_get_drvdata(info->dev);
>  	unsigned int rval;
>  	int ret;
>  
> @@ -75,8 +76,10 @@ static int mp2629_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		if (chan->channel == MP2629_INPUT_VOLT)
> +		if (chan->channel == MP2629_INPUT_VOLT &&
> +		    ddata->chip_id == CHIP_ID_MP2629)
>  			rval &= GENMASK(6, 0);
> +
>  		*val = rval;
>  		return IIO_VAL_INT;
>
Sebastian Reichel Oct. 29, 2022, 10:02 p.m. UTC | #4
Hi,

On Sat, Oct 29, 2022 at 11:29:52AM +0200, Saravanan Sekar wrote:
> add support for mp2733 Battery charger control driver for
> Monolithic Power System's MP2733 chipset.

I suppose you do not actually want to get this merged concidering
you did not carry over the Acked-by you got in v4? :)

-- Sebastian
Saravanan Sekar Oct. 29, 2022, 10:11 p.m. UTC | #5
On 30/10/22 00:02, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Oct 29, 2022 at 11:29:52AM +0200, Saravanan Sekar wrote:
>> add support for mp2733 Battery charger control driver for
>> Monolithic Power System's MP2733 chipset.
> 
> I suppose you do not actually want to get this merged concidering
> you did not carry over the Acked-by you got in v4? :)
> 
> -- Sebastian

Sorry, my mistake I added your ack to v5-0005 instead of v5-0007

Thanks,
Saravanan
Krzysztof Kozlowski Nov. 4, 2022, 1:54 a.m. UTC | #6
On 29/10/2022 05:29, Saravanan Sekar wrote:
> changes in v5:
>   - fixed commit message on v5-0002 and v5-0004
> 
> changes in v4:
>   - fixed attributes groups review comments in v3
>   - added new bug fix patches v4-0007 and v4-0008 
> 
> changes in v3:
>   - fixed dt_binding_check error
>   - fixed spelling usb->USB
> 
> changes in v2:
>   - fixed spelling
>   - revert back probe to probe_new in mfd driver
> 
> I do not see a cover letter, but FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

This does not belong to the cover letter. Please add respective tags to
patches, where applicable. If Andy gave Rb tag for entire patchset, add
it to the patches.

> for all patches except DT binding
> Note, some of the comments regarding spelling were given, I believe
> you are going to address them in v3.

...and this comment is from who? Andy?

> 
> 
> add support for mp2733 Battery charger control driver for Monolithic
> Power System's MP2733 chipset 
> 
> Saravanan Sekar (8):
>   iio: adc: mp2629: fix wrong comparison of channel
>   mfd: mp2629: fix failed to get iio channel by device name
>   iio: adc: mp2629: fix potential array out of bound access
>   power: supply: fix wrong interpretation of register value
>   mfd: mp2629: Add support for mps mp2733 battery charger
>   iio: adc: mp2629: restrict input voltage mask for mp2629
>   power: supply: Add support for mp2733 battery charger
>   power: supply: mp2629: Add USB fast charge settings
> 
>  .../ABI/testing/sysfs-class-power-mp2629      |  16 ++
>  drivers/iio/adc/mp2629_adc.c                  |   8 +-
>  drivers/mfd/mp2629.c                          |   7 +-
>  drivers/power/supply/mp2629_charger.c         | 229 +++++++++++++++---
>  include/linux/mfd/mp2629.h                    |   6 +

Why do you Cc DT maintainers?

>  5 files changed, 228 insertions(+), 38 deletions(-)
> 

Best regards,
Krzysztof
Saravanan Sekar Nov. 4, 2022, 7:13 a.m. UTC | #7
On 04/11/22 02:54, Krzysztof Kozlowski wrote:
> On 29/10/2022 05:29, Saravanan Sekar wrote:
>> changes in v5:
>>    - fixed commit message on v5-0002 and v5-0004
>>
>> changes in v4:
>>    - fixed attributes groups review comments in v3
>>    - added new bug fix patches v4-0007 and v4-0008
>>
>> changes in v3:
>>    - fixed dt_binding_check error
>>    - fixed spelling usb->USB
>>
>> changes in v2:
>>    - fixed spelling
>>    - revert back probe to probe_new in mfd driver
>>
>> I do not see a cover letter, but FWIW,
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> This does not belong to the cover letter. Please add respective tags to
> patches, where applicable. If Andy gave Rb tag for entire patchset, add
> it to the patches.
>

Hello Krzysztof,

These are v1 comments from Andy to me, all of them are addressed and I 
kept in cover letter for history

>> for all patches except DT binding
>> Note, some of the comments regarding spelling were given, I believe
>> you are going to address them in v3.
> 
> ...and this comment is from who? Andy?
> 
>>
>>
>> add support for mp2733 Battery charger control driver for Monolithic
>> Power System's MP2733 chipset
>>
>> Saravanan Sekar (8):
>>    iio: adc: mp2629: fix wrong comparison of channel
>>    mfd: mp2629: fix failed to get iio channel by device name
>>    iio: adc: mp2629: fix potential array out of bound access
>>    power: supply: fix wrong interpretation of register value
>>    mfd: mp2629: Add support for mps mp2733 battery charger
>>    iio: adc: mp2629: restrict input voltage mask for mp2629
>>    power: supply: Add support for mp2733 battery charger
>>    power: supply: mp2629: Add USB fast charge settings
>>
>>   .../ABI/testing/sysfs-class-power-mp2629      |  16 ++
>>   drivers/iio/adc/mp2629_adc.c                  |   8 +-
>>   drivers/mfd/mp2629.c                          |   7 +-
>>   drivers/power/supply/mp2629_charger.c         | 229 +++++++++++++++---
>>   include/linux/mfd/mp2629.h                    |   6 +
> 
> Why do you Cc DT maintainers?
>

This patch series includes DT bindings documentation which has already 
merged by 15Jun2022.

https://lore.kernel.org/all/20220615145357.2370044-3-sravanhome@gmail.com/

>>   5 files changed, 228 insertions(+), 38 deletions(-)
>>
> 
> Best regards,
> Krzysztof
> 

Thanks,
Saravanan
Krzysztof Kozlowski Nov. 4, 2022, 12:58 p.m. UTC | #8
On 04/11/2022 03:13, saravanan sekar wrote:
>>>   .../ABI/testing/sysfs-class-power-mp2629      |  16 ++
>>>   drivers/iio/adc/mp2629_adc.c                  |   8 +-
>>>   drivers/mfd/mp2629.c                          |   7 +-
>>>   drivers/power/supply/mp2629_charger.c         | 229 +++++++++++++++---
>>>   include/linux/mfd/mp2629.h                    |   6 +
>>
>> Why do you Cc DT maintainers?
>>
> 
> This patch series includes DT bindings documentation which has already 
> merged by 15Jun2022.

I don't see the DT bindings patch in above log. I did not get it and it
is not here:

https://lore.kernel.org/all/3e1b8549-0961-697b-63b8-db6b37d53c6b@gmail.com/

> 
> https://lore.kernel.org/all/20220615145357.2370044-3-sravanhome@gmail.com/

This is v3, how is it related?

Best regards,
Krzysztof