Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
Mark Brown writes: > On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote: > >> When charging is disabled (e.g. by removing the USB cable) the charge >> current is not reset to zero. So if I expose the current by the >> CURRENT_NOW property, it incorrectly reports the current that was set >> when charging was enabled, although there is no charging going on >> anymore. So I felt the need to update the charge current every time the >> charger gets enabled or disabled. >> Initially, the charge current is set to zero, so I think it needs to be >> set at least at the beginning to enable charging. > > Are you sure that the register value you're looking at is the actual > charge current right now and not just the maximum that the charger will > try to use depending on the conditions (supply available, battery > state...)? It seems like you're acting as though it's the latter but > that's not what the chip is doing. I was looking at the vendor code that was released for the Galaxy S3 and there the same register gets accessed for getting the current for the CURRENT_NOW property [1] and for setting the current [2]. So is this probably the wrong use of the CURRENT_NOW property because not the actual charge current is read but the maximum value that was set? Unfortunately, I don't have access to the datasheet and I didn't find it online so I don't know where the actual current can be accessed. Thanks, Wolfgang [1] https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n531 [2] https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n552 -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc signature.asc Description: PGP signature
Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
Mark Brown writes: > On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote: > >> When charging is disabled (e.g. by removing the USB cable) the charge >> current is not reset to zero. So if I expose the current by the >> CURRENT_NOW property, it incorrectly reports the current that was set >> when charging was enabled, although there is no charging going on >> anymore. So I felt the need to update the charge current every time the >> charger gets enabled or disabled. >> Initially, the charge current is set to zero, so I think it needs to be >> set at least at the beginning to enable charging. > > Are you sure that the register value you're looking at is the actual > charge current right now and not just the maximum that the charger will > try to use depending on the conditions (supply available, battery > state...)? It seems like you're acting as though it's the latter but > that's not what the chip is doing. I was looking at the vendor code that was released for the Galaxy S3 and there the same register gets accessed for getting the current for the CURRENT_NOW property [1] and for setting the current [2]. So is this probably the wrong use of the CURRENT_NOW property because not the actual charge current is read but the maximum value that was set? Unfortunately, I don't have access to the datasheet and I didn't find it online so I don't know where the actual current can be accessed. Thanks, Wolfgang [1] https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n531 [2] https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n552 -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc signature.asc Description: PGP signature
Re: [PATCH 1/3] mfd: max77693: Add defines for charger current control
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:08AM +0200, Wolfgang Wiedmeyer wrote:
>> This prepares for an updated regulator and charger driver. The defines
>> are needed to set the maximum input current and the fast charge
>> current.
>>
>> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>
> This shouldn't be separate patch, because:
> 1. You are touching later power and regulator so this patch should be
> applied to one and then pulled by other... a lot of unneeded work.
> 2. It is difficult to find which new code is needed for which driver.
>
> Simpler approach might work - when changing the regulator or psy driver,
> just change the header as needed. Unless there are conflicts and both of
> them need the same?
No, no conflicts. I will change the header as needed and won't do a
separate patch.
Thanks,
Wolfgang
> Best regards,
> Krzysztof
>
>> ---
>> include/linux/mfd/max77693-private.h | 7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/include/linux/mfd/max77693-private.h
>> b/include/linux/mfd/max77693-private.h
>> index 3c7a63b..ca18344 100644
>> --- a/include/linux/mfd/max77693-private.h
>> +++ b/include/linux/mfd/max77693-private.h
>> @@ -222,6 +222,9 @@ enum max77693_charger_battery_state {
>> #define CHG_CNFG_00_CHG_MASK0x1
>> #define CHG_CNFG_00_BUCK_MASK 0x4
>>
>> +/* MAX77693 CHG_CNFG_02 register */
>> +#define CHG_CNFG_02_CC_MASK 0x3F
>> +
>> /* MAX77693_CHG_REG_CHG_CNFG_01 register */
>> #define CHG_CNFG_01_FCHGTIME_SHIFT 0
>> #define CHG_CNFG_01_CHGRSTRT_SHIFT 4
>> @@ -258,6 +261,10 @@ enum max77693_charger_battery_state {
>>
>> /* MAX77693 CHG_CNFG_09 Register */
>> #define CHG_CNFG_09_CHGIN_ILIM_MASK 0x7F
>> +#define CHG_CNFG_09_CHGIN_ILIM_500_MAX 50
>> +#define CHG_CNFG_09_CHGIN_ILIM_500_MIN 47
>> +#define CHG_CNFG_09_CHGIN_ILIM_0_MAX6
>> +#define CHG_CNFG_09_CHGIN_ILIM_0_MIN0
>>
>> /* MAX77693 CHG_CTRL Register */
>> #define SAFEOUT_CTRL_SAFEOUT1_MASK 0x3
>> --
>> 2.8.0.rc3
>>
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH 1/3] mfd: max77693: Add defines for charger current control
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:08AM +0200, Wolfgang Wiedmeyer wrote:
>> This prepares for an updated regulator and charger driver. The defines
>> are needed to set the maximum input current and the fast charge
>> current.
>>
>> Signed-off-by: Wolfgang Wiedmeyer
>
> This shouldn't be separate patch, because:
> 1. You are touching later power and regulator so this patch should be
> applied to one and then pulled by other... a lot of unneeded work.
> 2. It is difficult to find which new code is needed for which driver.
>
> Simpler approach might work - when changing the regulator or psy driver,
> just change the header as needed. Unless there are conflicts and both of
> them need the same?
No, no conflicts. I will change the header as needed and won't do a
separate patch.
Thanks,
Wolfgang
> Best regards,
> Krzysztof
>
>> ---
>> include/linux/mfd/max77693-private.h | 7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/include/linux/mfd/max77693-private.h
>> b/include/linux/mfd/max77693-private.h
>> index 3c7a63b..ca18344 100644
>> --- a/include/linux/mfd/max77693-private.h
>> +++ b/include/linux/mfd/max77693-private.h
>> @@ -222,6 +222,9 @@ enum max77693_charger_battery_state {
>> #define CHG_CNFG_00_CHG_MASK0x1
>> #define CHG_CNFG_00_BUCK_MASK 0x4
>>
>> +/* MAX77693 CHG_CNFG_02 register */
>> +#define CHG_CNFG_02_CC_MASK 0x3F
>> +
>> /* MAX77693_CHG_REG_CHG_CNFG_01 register */
>> #define CHG_CNFG_01_FCHGTIME_SHIFT 0
>> #define CHG_CNFG_01_CHGRSTRT_SHIFT 4
>> @@ -258,6 +261,10 @@ enum max77693_charger_battery_state {
>>
>> /* MAX77693 CHG_CNFG_09 Register */
>> #define CHG_CNFG_09_CHGIN_ILIM_MASK 0x7F
>> +#define CHG_CNFG_09_CHGIN_ILIM_500_MAX 50
>> +#define CHG_CNFG_09_CHGIN_ILIM_500_MIN 47
>> +#define CHG_CNFG_09_CHGIN_ILIM_0_MAX6
>> +#define CHG_CNFG_09_CHGIN_ILIM_0_MIN0
>>
>> /* MAX77693 CHG_CTRL Register */
>> #define SAFEOUT_CTRL_SAFEOUT1_MASK 0x3
>> --
>> 2.8.0.rc3
>>
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
>> For MAX77693, the fast charge current also needs to be manipulated for
>> proper charging. The fast charge current is only set in the case of
>> the MAX77693 type, as the MAX77843 properly manipulates the fast
>> charge current.
>
> Are you sure it has to be manipulated? Some time I didn't dig into this.
> Now I looked at the datasheet and it says that usually there is no need
> for changing the charge current during the operation. Maxim recommends
> to setting it to a maximum safe value for the battery. The device will
> manage charge current on its own.
>
> However I agree that the charge current should be set... maybe once, to
> a maximum value appropriate for battery. Probably this should be done
> by max77693_charger in max77693_dt_init().
When charging is disabled (e.g. by removing the USB cable) the charge
current is not reset to zero. So if I expose the current by the
CURRENT_NOW property, it incorrectly reports the current that was set
when charging was enabled, although there is no charging going on
anymore. So I felt the need to update the charge current every time the
charger gets enabled or disabled.
Initially, the charge current is set to zero, so I think it needs to be
set at least at the beginning to enable charging.
Thanks,
Wolfgang
> Best regards,
> Krzysztof
>
>
>> The fast charge current is set to the next possible value below the
>> maximum input current.
>
>
>>
>> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>> ---
>> drivers/regulator/max77693-regulator.c | 45
>> +++---
>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/regulator/max77693-regulator.c
>> b/drivers/regulator/max77693-regulator.c
>> index cfbb951..e2f7584 100644
>> --- a/drivers/regulator/max77693-regulator.c
>> +++ b/drivers/regulator/max77693-regulator.c
>> @@ -54,14 +54,19 @@ struct chg_reg_data {
>> unsigned int linear_mask;
>> unsigned int uA_step;
>> unsigned int min_sel;
>> +
>> +bool set_fast;
>> +unsigned int fast_reg;
>> +unsigned int fast_mask;
>> };
>>
>> /*
>> * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>> * 0x00, 0x01, 0x2, 0x03= 60 mA
>> * 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
>> - * Actually for MAX77693 the driver manipulates the maximum input current,
>> - * not the fast charge current (output). This should be fixed.
>> + * Actually for MAX77693 the driver manipulates the maximum input current
>> + * and the fast charge current (output) because the fast charge current
>> + * is not set.
>> *
>> * On MAX77843 the calculation formula is the same (except values).
>> * Fortunately it properly manipulates the fast charge current.
>> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct
>> regulator_dev *rdev,
>> const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>> unsigned int chg_min_uA = rdev->constraints->min_uA;
>> int sel = 0;
>> +unsigned int data;
>> +int ret;
>>
>> while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>> sel++;
>> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct
>> regulator_dev *rdev,
>> /* the first four codes for charger current are all 60mA */
>> sel += reg_data->min_sel;
>>
>> -return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (reg_data->set_fast) {
>> +/* disable fast charge if minimum value */
>> +if (sel == reg_data->min_sel)
>> +data = 0;
>> +else {
>> +/*
>> + * set the fast charge current to the closest value
>> + * below the input current
>> + */
>> +ret = regmap_read(rdev->regmap, reg_data->fast_reg,
>> + );
>> +if (ret < 0)
>> +return ret;
>> +
>> +sel *= reg_data->uA_step / 1000; /* convert to mA */
>> +data &= ~reg_data->fast_mask;
>> +data |= sel * 10 / 333; /* 0.1A/3 steps */
>>
Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
>> For MAX77693, the fast charge current also needs to be manipulated for
>> proper charging. The fast charge current is only set in the case of
>> the MAX77693 type, as the MAX77843 properly manipulates the fast
>> charge current.
>
> Are you sure it has to be manipulated? Some time I didn't dig into this.
> Now I looked at the datasheet and it says that usually there is no need
> for changing the charge current during the operation. Maxim recommends
> to setting it to a maximum safe value for the battery. The device will
> manage charge current on its own.
>
> However I agree that the charge current should be set... maybe once, to
> a maximum value appropriate for battery. Probably this should be done
> by max77693_charger in max77693_dt_init().
When charging is disabled (e.g. by removing the USB cable) the charge
current is not reset to zero. So if I expose the current by the
CURRENT_NOW property, it incorrectly reports the current that was set
when charging was enabled, although there is no charging going on
anymore. So I felt the need to update the charge current every time the
charger gets enabled or disabled.
Initially, the charge current is set to zero, so I think it needs to be
set at least at the beginning to enable charging.
Thanks,
Wolfgang
> Best regards,
> Krzysztof
>
>
>> The fast charge current is set to the next possible value below the
>> maximum input current.
>
>
>>
>> Signed-off-by: Wolfgang Wiedmeyer
>> ---
>> drivers/regulator/max77693-regulator.c | 45
>> +++---
>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/regulator/max77693-regulator.c
>> b/drivers/regulator/max77693-regulator.c
>> index cfbb951..e2f7584 100644
>> --- a/drivers/regulator/max77693-regulator.c
>> +++ b/drivers/regulator/max77693-regulator.c
>> @@ -54,14 +54,19 @@ struct chg_reg_data {
>> unsigned int linear_mask;
>> unsigned int uA_step;
>> unsigned int min_sel;
>> +
>> +bool set_fast;
>> +unsigned int fast_reg;
>> +unsigned int fast_mask;
>> };
>>
>> /*
>> * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>> * 0x00, 0x01, 0x2, 0x03= 60 mA
>> * 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
>> - * Actually for MAX77693 the driver manipulates the maximum input current,
>> - * not the fast charge current (output). This should be fixed.
>> + * Actually for MAX77693 the driver manipulates the maximum input current
>> + * and the fast charge current (output) because the fast charge current
>> + * is not set.
>> *
>> * On MAX77843 the calculation formula is the same (except values).
>> * Fortunately it properly manipulates the fast charge current.
>> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct
>> regulator_dev *rdev,
>> const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>> unsigned int chg_min_uA = rdev->constraints->min_uA;
>> int sel = 0;
>> +unsigned int data;
>> +int ret;
>>
>> while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>> sel++;
>> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct
>> regulator_dev *rdev,
>> /* the first four codes for charger current are all 60mA */
>> sel += reg_data->min_sel;
>>
>> -return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (reg_data->set_fast) {
>> +/* disable fast charge if minimum value */
>> +if (sel == reg_data->min_sel)
>> +data = 0;
>> +else {
>> +/*
>> + * set the fast charge current to the closest value
>> + * below the input current
>> + */
>> +ret = regmap_read(rdev->regmap, reg_data->fast_reg,
>> + );
>> +if (ret < 0)
>> +return ret;
>> +
>> +sel *= reg_data->uA_step / 1000; /* convert to mA */
>> +data &= ~reg_data->fast_mask;
>> +data |= sel * 10 / 333; /* 0.1A/3 steps */
>> +}
>> +
>&
Re: [PATCH 3/3] power_supply: max77693: Listen for cable events and enable charging
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:10AM +0200, Wolfgang Wiedmeyer wrote:
>> This patch adds a listener for extcon cable events and enables
>> charging if an USB cable is connected. It recognizes SDP and DCP cable
>> types and treats them the same (same input current and fast charge
>> current). The maximum input current is set before the charger is
>> enabled and before the charger gets disabled, the maximum input
>> current is set to zero. The listener is inspired by the listener
>> implementation that was used for the AXP288 Charger driver.
>>
>> The patch also adds support for the CURRENT_NOW property. It reads the
>> fast charge current that gets set before the charger is enabled or
>> disabled.
>>
>> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>
> No. This power supply driver should not manage regulators. It is not a
> regulator consumer. For that specific need, there is a charger-manager driver.
When I was in the middle of implementing this, I noticed that the
charger manager does everything that is needed. But it took me quite
some time to configure the DTS correctly until I realized that the
charger manager used a deprecated function
(extcon_register_interest()) and thus couldn't work. And as I didn't see
the charger-manager in any other device's DTS, I thought that this might
not be right way.
But Chanwoo Choi has a fix: https://patchwork.kernel.org/patch/8898541/
So I will try to get it working with this patch.
> I agree that you might configure here the charger. You might even expose
> some writeable properties through power supply class. However the
> purpose of this driver is to expose the battery charger to user-space,
> not to replace the user-space with its work.
>
> So... NACK.
Ok, then I will try to reduce the patch to the CURRENT_NOW property
support.
> If you would like to play with charger-manager, here is my old DTS for
> Trats2 (might need updates):
Is there a reason that this patch is not in the kernel? It would have
been very helpful for me :)
Thanks,
Wolfgang
> index 595ad4ba6977..b4361b4a9de7 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -856,6 +856,44 @@
> };
> };
>
> + charger-manager@0 {
> + compatible = "charger-manager";
> + status = "okay";
> + chg-reg-supply = <_reg>;
> +
> + cm-name = "battery";
> + /* Polling only for external power source */
> + cm-poll-mode = <2>;
> + cm-poll-interval = <3>;
> +
> + cm-fullbatt-vchkdrop-ms = <3>;
> + cm-fullbatt-vchkdrop-volt = <15>;
> + cm-fullbatt-soc = <100>;
> +
> + cm-battery-stat = <0>;
> + cm-fuel-gauge = "max170xx_battery";
> +
> + /* Allow charging for 5hr */
> + cm-charging-max = <1800>;
> + /* Allow discharging for 2hr */
> + cm-discharging-max = <720>;
> +
> + cm-num-chargers = <1>;
> + cm-chargers = "max77693-charger";
> +
> + charger@0 {
> + cm-regulator-name = "chg-reg";
> + cable@0 {
> + cm-cable-name = "USB";
> + cm-cable-extcon = "max77693-muic";
> + };
> + cable@1 {
> + cm-cable-name = "TA";
> + cm-cable-extcon = "max77693-muic";
> + };
> + };
> + };
> +
> exynos-usbphy@125B {
> status = "okay";
> };
>
> Signed-off-by: Krzysztof Kozlowski <k...@kernel.org>
>
> Best regards,
> Krzysztof
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH 3/3] power_supply: max77693: Listen for cable events and enable charging
Krzysztof Kozlowski writes:
> On Tue, Sep 27, 2016 at 01:31:10AM +0200, Wolfgang Wiedmeyer wrote:
>> This patch adds a listener for extcon cable events and enables
>> charging if an USB cable is connected. It recognizes SDP and DCP cable
>> types and treats them the same (same input current and fast charge
>> current). The maximum input current is set before the charger is
>> enabled and before the charger gets disabled, the maximum input
>> current is set to zero. The listener is inspired by the listener
>> implementation that was used for the AXP288 Charger driver.
>>
>> The patch also adds support for the CURRENT_NOW property. It reads the
>> fast charge current that gets set before the charger is enabled or
>> disabled.
>>
>> Signed-off-by: Wolfgang Wiedmeyer
>
> No. This power supply driver should not manage regulators. It is not a
> regulator consumer. For that specific need, there is a charger-manager driver.
When I was in the middle of implementing this, I noticed that the
charger manager does everything that is needed. But it took me quite
some time to configure the DTS correctly until I realized that the
charger manager used a deprecated function
(extcon_register_interest()) and thus couldn't work. And as I didn't see
the charger-manager in any other device's DTS, I thought that this might
not be right way.
But Chanwoo Choi has a fix: https://patchwork.kernel.org/patch/8898541/
So I will try to get it working with this patch.
> I agree that you might configure here the charger. You might even expose
> some writeable properties through power supply class. However the
> purpose of this driver is to expose the battery charger to user-space,
> not to replace the user-space with its work.
>
> So... NACK.
Ok, then I will try to reduce the patch to the CURRENT_NOW property
support.
> If you would like to play with charger-manager, here is my old DTS for
> Trats2 (might need updates):
Is there a reason that this patch is not in the kernel? It would have
been very helpful for me :)
Thanks,
Wolfgang
> index 595ad4ba6977..b4361b4a9de7 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -856,6 +856,44 @@
> };
> };
>
> + charger-manager@0 {
> + compatible = "charger-manager";
> + status = "okay";
> + chg-reg-supply = <_reg>;
> +
> + cm-name = "battery";
> + /* Polling only for external power source */
> + cm-poll-mode = <2>;
> + cm-poll-interval = <3>;
> +
> + cm-fullbatt-vchkdrop-ms = <3>;
> + cm-fullbatt-vchkdrop-volt = <15>;
> + cm-fullbatt-soc = <100>;
> +
> + cm-battery-stat = <0>;
> + cm-fuel-gauge = "max170xx_battery";
> +
> + /* Allow charging for 5hr */
> + cm-charging-max = <1800>;
> + /* Allow discharging for 2hr */
> + cm-discharging-max = <720>;
> +
> + cm-num-chargers = <1>;
> + cm-chargers = "max77693-charger";
> +
> + charger@0 {
> + cm-regulator-name = "chg-reg";
> + cable@0 {
> + cm-cable-name = "USB";
> + cm-cable-extcon = "max77693-muic";
> + };
> + cable@1 {
> + cm-cable-name = "TA";
> + cm-cable-extcon = "max77693-muic";
> + };
> + };
> + };
> +
> exynos-usbphy@125B {
> status = "okay";
> };
>
> Signed-off-by: Krzysztof Kozlowski
>
> Best regards,
> Krzysztof
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
[PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
For MAX77693, the fast charge current also needs to be manipulated for
proper charging. The fast charge current is only set in the case of
the MAX77693 type, as the MAX77843 properly manipulates the fast
charge current.
The fast charge current is set to the next possible value below the
maximum input current.
Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
---
drivers/regulator/max77693-regulator.c | 45 +++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/max77693-regulator.c
b/drivers/regulator/max77693-regulator.c
index cfbb951..e2f7584 100644
--- a/drivers/regulator/max77693-regulator.c
+++ b/drivers/regulator/max77693-regulator.c
@@ -54,14 +54,19 @@ struct chg_reg_data {
unsigned int linear_mask;
unsigned int uA_step;
unsigned int min_sel;
+
+ bool set_fast;
+ unsigned int fast_reg;
+ unsigned int fast_mask;
};
/*
* MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
* 0x00, 0x01, 0x2, 0x03 = 60 mA
* 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
- * Actually for MAX77693 the driver manipulates the maximum input current,
- * not the fast charge current (output). This should be fixed.
+ * Actually for MAX77693 the driver manipulates the maximum input current
+ * and the fast charge current (output) because the fast charge current
+ * is not set.
*
* On MAX77843 the calculation formula is the same (except values).
* Fortunately it properly manipulates the fast charge current.
@@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct
regulator_dev *rdev,
const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
unsigned int chg_min_uA = rdev->constraints->min_uA;
int sel = 0;
+ unsigned int data;
+ int ret;
while (chg_min_uA + reg_data->uA_step * sel < min_uA)
sel++;
@@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct
regulator_dev *rdev,
/* the first four codes for charger current are all 60mA */
sel += reg_data->min_sel;
- return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+ ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+ if (ret < 0)
+ return ret;
+
+ if (reg_data->set_fast) {
+ /* disable fast charge if minimum value */
+ if (sel == reg_data->min_sel)
+ data = 0;
+ else {
+ /*
+* set the fast charge current to the closest value
+* below the input current
+*/
+ ret = regmap_read(rdev->regmap, reg_data->fast_reg,
+ );
+ if (ret < 0)
+ return ret;
+
+ sel *= reg_data->uA_step / 1000; /* convert to mA */
+ data &= ~reg_data->fast_mask;
+ data |= sel * 10 / 333; /* 0.1A/3 steps */
+ }
+
+ ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
}
/* end of CHARGER regulator ops */
@@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
.linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
.uA_step= 2,
.min_sel= 3,
+ .set_fast = true,
+ .fast_reg = MAX77693_CHG_REG_CHG_CNFG_02,
+ .fast_mask = CHG_CNFG_02_CC_MASK,
};
#definemax77843_regulator_desc_esafeout(num) {
\
@@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
.linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
.uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
.min_sel= 2,
+ .set_fast = false,
};
static int max77693_pmic_probe(struct platform_device *pdev)
--
2.8.0.rc3
[PATCH 3/3] power_supply: max77693: Listen for cable events and enable charging
This patch adds a listener for extcon cable events and enables
charging if an USB cable is connected. It recognizes SDP and DCP cable
types and treats them the same (same input current and fast charge
current). The maximum input current is set before the charger is
enabled and before the charger gets disabled, the maximum input
current is set to zero. The listener is inspired by the listener
implementation that was used for the AXP288 Charger driver.
The patch also adds support for the CURRENT_NOW property. It reads the
fast charge current that gets set before the charger is enabled or
disabled.
Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
---
drivers/power/Kconfig| 2 +-
drivers/power/max77693_charger.c | 171 +++
2 files changed, 172 insertions(+), 1 deletion(-)
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..28254d3 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -372,7 +372,7 @@ config CHARGER_MAX14577
config CHARGER_MAX77693
tristate "Maxim MAX77693 battery charger driver"
- depends on MFD_MAX77693
+ depends on MFD_MAX77693 && REGULATOR_MAX77693
help
Say Y to enable support for the Maxim MAX77693 battery charger.
diff --git a/drivers/power/max77693_charger.c b/drivers/power/max77693_charger.c
index 060cab5..922f43f 100644
--- a/drivers/power/max77693_charger.c
+++ b/drivers/power/max77693_charger.c
@@ -22,8 +22,11 @@
#include
#include
#include
+#include
+#include
#define MAX77693_CHARGER_NAME "max77693-charger"
+#define MAX77693_EXTCON_DEV_NAME "max77693-muic"
static const char *max77693_charger_model = "MAX77693";
static const char *max77693_charger_manufacturer = "Maxim Integrated";
@@ -31,12 +34,21 @@ struct max77693_charger {
struct device *dev;
struct max77693_dev *max77693;
struct power_supply *charger;
+ struct regulator*regu;
u32 constant_volt;
u32 min_system_volt;
u32 thermal_regulation_temp;
u32 batttery_overcurrent;
u32 charge_input_threshold_volt;
+
+ /* SDP/DCP USB charging cable notifications */
+ struct {
+ struct extcon_dev *edev;
+ bool connected;
+ struct notifier_block nb;
+ struct work_struct work;
+ } cable;
};
static int max77693_get_charger_state(struct regmap *regmap, int *val)
@@ -207,12 +219,28 @@ static int max77693_get_online(struct regmap *regmap, int
*val)
return 0;
}
+int max77693_get_charge_current(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, MAX77693_CHG_REG_CHG_CNFG_02, );
+ if (ret < 0)
+ return ret;
+
+ data &= CHG_CNFG_02_CC_MASK;
+ *val = data * 333 / 10; /* 3 steps/0.1A */
+
+ return 0;
+}
+
static enum power_supply_property max77693_charger_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_CHARGE_TYPE,
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_MANUFACTURER,
};
@@ -241,6 +269,9 @@ static int max77693_charger_get_property(struct
power_supply *psy,
case POWER_SUPPLY_PROP_ONLINE:
ret = max77693_get_online(regmap, >intval);
break;
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = max77693_get_charge_current(regmap, >intval);
+ break;
case POWER_SUPPLY_PROP_MODEL_NAME:
val->strval = max77693_charger_model;
break;
@@ -295,6 +326,7 @@ static ssize_t fast_charge_timer_show(struct device *dev,
data &= CHG_CNFG_01_FCHGTIME_MASK;
data >>= CHG_CNFG_01_FCHGTIME_SHIFT;
+
switch (data) {
case 0x1 ... 0x7:
/* Starting from 4 hours, step by 2 hours */
@@ -582,6 +614,97 @@ static int max77693_set_charge_input_threshold_volt(struct
max77693_charger *chg
CHG_CNFG_12_VCHGINREG_MASK, data);
}
+static int max77693_enable_charger(struct max77693_charger *chg, bool enable)
+{
+ int ret;
+
+ if (enable) {
+ regulator_set_current_limit(chg->regu,
+ CHG_CNFG_09_CHGIN_ILIM_500_MIN,
+ CHG_CNFG_09_CHGIN_ILIM_500_MAX);
+ if (ret < 0)
+ return ret;
+
+ ret = regulator_enable(chg->regu);
+ if (ret < 0)
+ return ret;
+ } else {
+
[PATCH 2/3] regulator: max77693: Also manipulate the fast charge current
For MAX77693, the fast charge current also needs to be manipulated for
proper charging. The fast charge current is only set in the case of
the MAX77693 type, as the MAX77843 properly manipulates the fast
charge current.
The fast charge current is set to the next possible value below the
maximum input current.
Signed-off-by: Wolfgang Wiedmeyer
---
drivers/regulator/max77693-regulator.c | 45 +++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/max77693-regulator.c
b/drivers/regulator/max77693-regulator.c
index cfbb951..e2f7584 100644
--- a/drivers/regulator/max77693-regulator.c
+++ b/drivers/regulator/max77693-regulator.c
@@ -54,14 +54,19 @@ struct chg_reg_data {
unsigned int linear_mask;
unsigned int uA_step;
unsigned int min_sel;
+
+ bool set_fast;
+ unsigned int fast_reg;
+ unsigned int fast_mask;
};
/*
* MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
* 0x00, 0x01, 0x2, 0x03 = 60 mA
* 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
- * Actually for MAX77693 the driver manipulates the maximum input current,
- * not the fast charge current (output). This should be fixed.
+ * Actually for MAX77693 the driver manipulates the maximum input current
+ * and the fast charge current (output) because the fast charge current
+ * is not set.
*
* On MAX77843 the calculation formula is the same (except values).
* Fortunately it properly manipulates the fast charge current.
@@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct
regulator_dev *rdev,
const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
unsigned int chg_min_uA = rdev->constraints->min_uA;
int sel = 0;
+ unsigned int data;
+ int ret;
while (chg_min_uA + reg_data->uA_step * sel < min_uA)
sel++;
@@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct
regulator_dev *rdev,
/* the first four codes for charger current are all 60mA */
sel += reg_data->min_sel;
- return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+ ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+ if (ret < 0)
+ return ret;
+
+ if (reg_data->set_fast) {
+ /* disable fast charge if minimum value */
+ if (sel == reg_data->min_sel)
+ data = 0;
+ else {
+ /*
+* set the fast charge current to the closest value
+* below the input current
+*/
+ ret = regmap_read(rdev->regmap, reg_data->fast_reg,
+ );
+ if (ret < 0)
+ return ret;
+
+ sel *= reg_data->uA_step / 1000; /* convert to mA */
+ data &= ~reg_data->fast_mask;
+ data |= sel * 10 / 333; /* 0.1A/3 steps */
+ }
+
+ ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
}
/* end of CHARGER regulator ops */
@@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
.linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
.uA_step= 2,
.min_sel= 3,
+ .set_fast = true,
+ .fast_reg = MAX77693_CHG_REG_CHG_CNFG_02,
+ .fast_mask = CHG_CNFG_02_CC_MASK,
};
#definemax77843_regulator_desc_esafeout(num) {
\
@@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
.linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
.uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
.min_sel= 2,
+ .set_fast = false,
};
static int max77693_pmic_probe(struct platform_device *pdev)
--
2.8.0.rc3
[PATCH 3/3] power_supply: max77693: Listen for cable events and enable charging
This patch adds a listener for extcon cable events and enables
charging if an USB cable is connected. It recognizes SDP and DCP cable
types and treats them the same (same input current and fast charge
current). The maximum input current is set before the charger is
enabled and before the charger gets disabled, the maximum input
current is set to zero. The listener is inspired by the listener
implementation that was used for the AXP288 Charger driver.
The patch also adds support for the CURRENT_NOW property. It reads the
fast charge current that gets set before the charger is enabled or
disabled.
Signed-off-by: Wolfgang Wiedmeyer
---
drivers/power/Kconfig| 2 +-
drivers/power/max77693_charger.c | 171 +++
2 files changed, 172 insertions(+), 1 deletion(-)
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..28254d3 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -372,7 +372,7 @@ config CHARGER_MAX14577
config CHARGER_MAX77693
tristate "Maxim MAX77693 battery charger driver"
- depends on MFD_MAX77693
+ depends on MFD_MAX77693 && REGULATOR_MAX77693
help
Say Y to enable support for the Maxim MAX77693 battery charger.
diff --git a/drivers/power/max77693_charger.c b/drivers/power/max77693_charger.c
index 060cab5..922f43f 100644
--- a/drivers/power/max77693_charger.c
+++ b/drivers/power/max77693_charger.c
@@ -22,8 +22,11 @@
#include
#include
#include
+#include
+#include
#define MAX77693_CHARGER_NAME "max77693-charger"
+#define MAX77693_EXTCON_DEV_NAME "max77693-muic"
static const char *max77693_charger_model = "MAX77693";
static const char *max77693_charger_manufacturer = "Maxim Integrated";
@@ -31,12 +34,21 @@ struct max77693_charger {
struct device *dev;
struct max77693_dev *max77693;
struct power_supply *charger;
+ struct regulator*regu;
u32 constant_volt;
u32 min_system_volt;
u32 thermal_regulation_temp;
u32 batttery_overcurrent;
u32 charge_input_threshold_volt;
+
+ /* SDP/DCP USB charging cable notifications */
+ struct {
+ struct extcon_dev *edev;
+ bool connected;
+ struct notifier_block nb;
+ struct work_struct work;
+ } cable;
};
static int max77693_get_charger_state(struct regmap *regmap, int *val)
@@ -207,12 +219,28 @@ static int max77693_get_online(struct regmap *regmap, int
*val)
return 0;
}
+int max77693_get_charge_current(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, MAX77693_CHG_REG_CHG_CNFG_02, );
+ if (ret < 0)
+ return ret;
+
+ data &= CHG_CNFG_02_CC_MASK;
+ *val = data * 333 / 10; /* 3 steps/0.1A */
+
+ return 0;
+}
+
static enum power_supply_property max77693_charger_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_CHARGE_TYPE,
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_MANUFACTURER,
};
@@ -241,6 +269,9 @@ static int max77693_charger_get_property(struct
power_supply *psy,
case POWER_SUPPLY_PROP_ONLINE:
ret = max77693_get_online(regmap, >intval);
break;
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = max77693_get_charge_current(regmap, >intval);
+ break;
case POWER_SUPPLY_PROP_MODEL_NAME:
val->strval = max77693_charger_model;
break;
@@ -295,6 +326,7 @@ static ssize_t fast_charge_timer_show(struct device *dev,
data &= CHG_CNFG_01_FCHGTIME_MASK;
data >>= CHG_CNFG_01_FCHGTIME_SHIFT;
+
switch (data) {
case 0x1 ... 0x7:
/* Starting from 4 hours, step by 2 hours */
@@ -582,6 +614,97 @@ static int max77693_set_charge_input_threshold_volt(struct
max77693_charger *chg
CHG_CNFG_12_VCHGINREG_MASK, data);
}
+static int max77693_enable_charger(struct max77693_charger *chg, bool enable)
+{
+ int ret;
+
+ if (enable) {
+ regulator_set_current_limit(chg->regu,
+ CHG_CNFG_09_CHGIN_ILIM_500_MIN,
+ CHG_CNFG_09_CHGIN_ILIM_500_MAX);
+ if (ret < 0)
+ return ret;
+
+ ret = regulator_enable(chg->regu);
+ if (ret < 0)
+ return ret;
+ } else {
+ /* sets fast charge current to zero */
+ ret = regulator_set_current
[PATCH 0/3] max77693: USB event listener for charger
This patch series adds an USB event listener to the charger driver that enables charging if an USB cable is connected. Before the charger can be enabled, the maximum input current and the fast charge current need to be set. In order to make this possible, the regulator function that sets the current needs to be extended. Wolfgang Wiedmeyer (3): mfd: max77693: Add defines for charger current control regulator: max77693: Also manipulate the fast charge current power_supply: max77693: Listen for cable events and enable charging drivers/power/Kconfig | 2 +- drivers/power/max77693_charger.c | 171 + drivers/regulator/max77693-regulator.c | 45 - include/linux/mfd/max77693-private.h | 7 ++ 4 files changed, 221 insertions(+), 4 deletions(-) -- 2.8.0.rc3
[PATCH 1/3] mfd: max77693: Add defines for charger current control
This prepares for an updated regulator and charger driver. The defines
are needed to set the maximum input current and the fast charge
current.
Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
---
include/linux/mfd/max77693-private.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/linux/mfd/max77693-private.h
b/include/linux/mfd/max77693-private.h
index 3c7a63b..ca18344 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -222,6 +222,9 @@ enum max77693_charger_battery_state {
#define CHG_CNFG_00_CHG_MASK 0x1
#define CHG_CNFG_00_BUCK_MASK 0x4
+/* MAX77693 CHG_CNFG_02 register */
+#define CHG_CNFG_02_CC_MASK0x3F
+
/* MAX77693_CHG_REG_CHG_CNFG_01 register */
#define CHG_CNFG_01_FCHGTIME_SHIFT 0
#define CHG_CNFG_01_CHGRSTRT_SHIFT 4
@@ -258,6 +261,10 @@ enum max77693_charger_battery_state {
/* MAX77693 CHG_CNFG_09 Register */
#define CHG_CNFG_09_CHGIN_ILIM_MASK0x7F
+#define CHG_CNFG_09_CHGIN_ILIM_500_MAX 50
+#define CHG_CNFG_09_CHGIN_ILIM_500_MIN 47
+#define CHG_CNFG_09_CHGIN_ILIM_0_MAX 6
+#define CHG_CNFG_09_CHGIN_ILIM_0_MIN 0
/* MAX77693 CHG_CTRL Register */
#define SAFEOUT_CTRL_SAFEOUT1_MASK 0x3
--
2.8.0.rc3
[PATCH 0/3] max77693: USB event listener for charger
This patch series adds an USB event listener to the charger driver that enables charging if an USB cable is connected. Before the charger can be enabled, the maximum input current and the fast charge current need to be set. In order to make this possible, the regulator function that sets the current needs to be extended. Wolfgang Wiedmeyer (3): mfd: max77693: Add defines for charger current control regulator: max77693: Also manipulate the fast charge current power_supply: max77693: Listen for cable events and enable charging drivers/power/Kconfig | 2 +- drivers/power/max77693_charger.c | 171 + drivers/regulator/max77693-regulator.c | 45 - include/linux/mfd/max77693-private.h | 7 ++ 4 files changed, 221 insertions(+), 4 deletions(-) -- 2.8.0.rc3
[PATCH 1/3] mfd: max77693: Add defines for charger current control
This prepares for an updated regulator and charger driver. The defines
are needed to set the maximum input current and the fast charge
current.
Signed-off-by: Wolfgang Wiedmeyer
---
include/linux/mfd/max77693-private.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/linux/mfd/max77693-private.h
b/include/linux/mfd/max77693-private.h
index 3c7a63b..ca18344 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -222,6 +222,9 @@ enum max77693_charger_battery_state {
#define CHG_CNFG_00_CHG_MASK 0x1
#define CHG_CNFG_00_BUCK_MASK 0x4
+/* MAX77693 CHG_CNFG_02 register */
+#define CHG_CNFG_02_CC_MASK0x3F
+
/* MAX77693_CHG_REG_CHG_CNFG_01 register */
#define CHG_CNFG_01_FCHGTIME_SHIFT 0
#define CHG_CNFG_01_CHGRSTRT_SHIFT 4
@@ -258,6 +261,10 @@ enum max77693_charger_battery_state {
/* MAX77693 CHG_CNFG_09 Register */
#define CHG_CNFG_09_CHGIN_ILIM_MASK0x7F
+#define CHG_CNFG_09_CHGIN_ILIM_500_MAX 50
+#define CHG_CNFG_09_CHGIN_ILIM_500_MIN 47
+#define CHG_CNFG_09_CHGIN_ILIM_0_MAX 6
+#define CHG_CNFG_09_CHGIN_ILIM_0_MIN 0
/* MAX77693 CHG_CTRL Register */
#define SAFEOUT_CTRL_SAFEOUT1_MASK 0x3
--
2.8.0.rc3
Re: [PATCH 1/2] power: supply: max17042_battery: use VF SOC register for capacity property
Krzysztof Kozlowski writes: > On Sun, Sep 25, 2016 at 11:10:10PM +0200, Wolfgang Wiedmeyer wrote: >> The capacity property uses the RepSOC register to report the current state >> of charge. This register did not provide a reliable SOC value during my >> testing with the max17047 variant on a Galaxy S3 (Trats2/GT-I9300). The >> reported value did not change or even stayed zero in some cases. >> However, the VF SOC register provided an accurate SOC value at all times. >> It uses the voltage fuel gauge to determine the SOC. >> >> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de> >> --- >> drivers/power/max17042_battery.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/power/max17042_battery.c >> b/drivers/power/max17042_battery.c >> index da7a75f..20cb1fd 100644 >> --- a/drivers/power/max17042_battery.c >> +++ b/drivers/power/max17042_battery.c >> @@ -246,7 +246,7 @@ static int max17042_get_property(struct power_supply >> *psy, >> val->intval = data * 625 / 8; >> break; >> case POWER_SUPPLY_PROP_CAPACITY: >> -ret = regmap_read(map, MAX17042_RepSOC, ); >> +ret = regmap_read(map, MAX17042_VFSOC, ); >> if (ret < 0) >> return ret; > > The RepSOC is for ModelGauge m3 which requires current sense resistor. I > don't remember whether the resistor is present on Trats2. If not, then > m1 is used. However in both cases (m1 and m3) the battery > characteristics (cell information) should be loaded which in case of DT > driver is not supported. > > Overall, I am not sure whether your change is correct. It might fix this > particular scenario because: > 1. We are not providing the cell information, > 2. We mre not providing the SNS resistor value so we are in m1 mode (if > there is no SNS resistor). but it might break other applications where > SNS is present and cell configuration is provided. Unless you tested it > in such? I'm not able to test other applications than the Galaxy S3. > Probably this should be based on Device Tree property describing what is > configured (e.g. missing model data). Maybe existing maxim,rsns-microohm > could be used - in case of lack of it, fall back to reading VFSOC? Ok, I'll include a check if maxim,rsns-microohm exists and do the fallback to VFSOC if it's not there. Thanks, Wolfgang -- Website: https://fossencdi.org Jabber: wolfg...@wiedmeyer.de OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc signature.asc Description: PGP signature
Re: [PATCH 1/2] power: supply: max17042_battery: use VF SOC register for capacity property
Krzysztof Kozlowski writes: > On Sun, Sep 25, 2016 at 11:10:10PM +0200, Wolfgang Wiedmeyer wrote: >> The capacity property uses the RepSOC register to report the current state >> of charge. This register did not provide a reliable SOC value during my >> testing with the max17047 variant on a Galaxy S3 (Trats2/GT-I9300). The >> reported value did not change or even stayed zero in some cases. >> However, the VF SOC register provided an accurate SOC value at all times. >> It uses the voltage fuel gauge to determine the SOC. >> >> Signed-off-by: Wolfgang Wiedmeyer >> --- >> drivers/power/max17042_battery.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/power/max17042_battery.c >> b/drivers/power/max17042_battery.c >> index da7a75f..20cb1fd 100644 >> --- a/drivers/power/max17042_battery.c >> +++ b/drivers/power/max17042_battery.c >> @@ -246,7 +246,7 @@ static int max17042_get_property(struct power_supply >> *psy, >> val->intval = data * 625 / 8; >> break; >> case POWER_SUPPLY_PROP_CAPACITY: >> -ret = regmap_read(map, MAX17042_RepSOC, ); >> +ret = regmap_read(map, MAX17042_VFSOC, ); >> if (ret < 0) >> return ret; > > The RepSOC is for ModelGauge m3 which requires current sense resistor. I > don't remember whether the resistor is present on Trats2. If not, then > m1 is used. However in both cases (m1 and m3) the battery > characteristics (cell information) should be loaded which in case of DT > driver is not supported. > > Overall, I am not sure whether your change is correct. It might fix this > particular scenario because: > 1. We are not providing the cell information, > 2. We mre not providing the SNS resistor value so we are in m1 mode (if > there is no SNS resistor). but it might break other applications where > SNS is present and cell configuration is provided. Unless you tested it > in such? I'm not able to test other applications than the Galaxy S3. > Probably this should be based on Device Tree property describing what is > configured (e.g. missing model data). Maybe existing maxim,rsns-microohm > could be used - in case of lack of it, fall back to reading VFSOC? Ok, I'll include a check if maxim,rsns-microohm exists and do the fallback to VFSOC if it's not there. Thanks, Wolfgang -- Website: https://fossencdi.org Jabber: wolfg...@wiedmeyer.de OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc signature.asc Description: PGP signature
Re: [PATCH 2/2] power: supply: max17042_battery: add technology property support
Krzysztof Kozlowski writes:
> On Sun, Sep 25, 2016 at 11:10:11PM +0200, Wolfgang Wiedmeyer wrote:
>> This patch reports the battery technology as Li-ion.
>>
>> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>> ---
>> drivers/power/max17042_battery.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/power/max17042_battery.c
>> b/drivers/power/max17042_battery.c
>> index 20cb1fd..43cb5df 100644
>> --- a/drivers/power/max17042_battery.c
>> +++ b/drivers/power/max17042_battery.c
>> @@ -92,6 +92,7 @@ static enum power_supply_property max17042_battery_props[]
>> = {
>> POWER_SUPPLY_PROP_TEMP_MIN,
>> POWER_SUPPLY_PROP_TEMP_MAX,
>> POWER_SUPPLY_PROP_HEALTH,
>> +POWER_SUPPLY_PROP_TECHNOLOGY,
>> POWER_SUPPLY_PROP_CURRENT_NOW,
>> POWER_SUPPLY_PROP_CURRENT_AVG,
>> };
>> @@ -296,6 +297,9 @@ static int max17042_get_property(struct power_supply
>> *psy,
>> if (ret < 0)
>> return ret;
>> break;
>> +case POWER_SUPPLY_PROP_TECHNOLOGY:
>> +val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
>
> How can you be sure it is always Li-Ion? For wearables and mobiles, rather
> yes, but
> the driver is also used in other devices. Technically, specs are saying
> it might be used also with Li-Poly applications.
I suppose that there is no way to detect this. Would it be ok if I add
an optional Device Tree property that allows to specify if it's Li-Ion
or Li-Poly? If the property is not supplied, then "unknown" is returned.
Thanks,
Wolfgang
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH 2/2] power: supply: max17042_battery: add technology property support
Krzysztof Kozlowski writes:
> On Sun, Sep 25, 2016 at 11:10:11PM +0200, Wolfgang Wiedmeyer wrote:
>> This patch reports the battery technology as Li-ion.
>>
>> Signed-off-by: Wolfgang Wiedmeyer
>> ---
>> drivers/power/max17042_battery.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/power/max17042_battery.c
>> b/drivers/power/max17042_battery.c
>> index 20cb1fd..43cb5df 100644
>> --- a/drivers/power/max17042_battery.c
>> +++ b/drivers/power/max17042_battery.c
>> @@ -92,6 +92,7 @@ static enum power_supply_property max17042_battery_props[]
>> = {
>> POWER_SUPPLY_PROP_TEMP_MIN,
>> POWER_SUPPLY_PROP_TEMP_MAX,
>> POWER_SUPPLY_PROP_HEALTH,
>> +POWER_SUPPLY_PROP_TECHNOLOGY,
>> POWER_SUPPLY_PROP_CURRENT_NOW,
>> POWER_SUPPLY_PROP_CURRENT_AVG,
>> };
>> @@ -296,6 +297,9 @@ static int max17042_get_property(struct power_supply
>> *psy,
>> if (ret < 0)
>> return ret;
>> break;
>> +case POWER_SUPPLY_PROP_TECHNOLOGY:
>> +val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
>
> How can you be sure it is always Li-Ion? For wearables and mobiles, rather
> yes, but
> the driver is also used in other devices. Technically, specs are saying
> it might be used also with Li-Poly applications.
I suppose that there is no way to detect this. Would it be ok if I add
an optional Device Tree property that allows to specify if it's Li-Ion
or Li-Poly? If the property is not supplied, then "unknown" is returned.
Thanks,
Wolfgang
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
[PATCH 2/2] power: supply: max17042_battery: add technology property support
This patch reports the battery technology as Li-ion.
Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
---
drivers/power/max17042_battery.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 20cb1fd..43cb5df 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -92,6 +92,7 @@ static enum power_supply_property max17042_battery_props[] = {
POWER_SUPPLY_PROP_TEMP_MIN,
POWER_SUPPLY_PROP_TEMP_MAX,
POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
POWER_SUPPLY_PROP_CURRENT_NOW,
POWER_SUPPLY_PROP_CURRENT_AVG,
};
@@ -296,6 +297,9 @@ static int max17042_get_property(struct power_supply *psy,
if (ret < 0)
return ret;
break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
if (chip->pdata->enable_current_sense) {
ret = regmap_read(map, MAX17042_Current, );
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH 0/2] power: supply: max17042_battery: Improve properties
These two patches fix the capacity property and add the technology property. Wolfgang Wiedmeyer (2): power: supply: max17042_battery: use VF SOC register for capacity property power: supply: max17042_battery: add technology property support drivers/power/max17042_battery.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH 1/2] power: supply: max17042_battery: use VF SOC register for capacity property
The capacity property uses the RepSOC register to report the current state of charge. This register did not provide a reliable SOC value during my testing with the max17047 variant on a Galaxy S3 (Trats2/GT-I9300). The reported value did not change or even stayed zero in some cases. However, the VF SOC register provided an accurate SOC value at all times. It uses the voltage fuel gauge to determine the SOC. Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de> --- drivers/power/max17042_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index da7a75f..20cb1fd 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -246,7 +246,7 @@ static int max17042_get_property(struct power_supply *psy, val->intval = data * 625 / 8; break; case POWER_SUPPLY_PROP_CAPACITY: - ret = regmap_read(map, MAX17042_RepSOC, ); + ret = regmap_read(map, MAX17042_VFSOC, ); if (ret < 0) return ret; -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH 2/2] power: supply: max17042_battery: add technology property support
This patch reports the battery technology as Li-ion.
Signed-off-by: Wolfgang Wiedmeyer
---
drivers/power/max17042_battery.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 20cb1fd..43cb5df 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -92,6 +92,7 @@ static enum power_supply_property max17042_battery_props[] = {
POWER_SUPPLY_PROP_TEMP_MIN,
POWER_SUPPLY_PROP_TEMP_MAX,
POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
POWER_SUPPLY_PROP_CURRENT_NOW,
POWER_SUPPLY_PROP_CURRENT_AVG,
};
@@ -296,6 +297,9 @@ static int max17042_get_property(struct power_supply *psy,
if (ret < 0)
return ret;
break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
if (chip->pdata->enable_current_sense) {
ret = regmap_read(map, MAX17042_Current, );
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH 0/2] power: supply: max17042_battery: Improve properties
These two patches fix the capacity property and add the technology property. Wolfgang Wiedmeyer (2): power: supply: max17042_battery: use VF SOC register for capacity property power: supply: max17042_battery: add technology property support drivers/power/max17042_battery.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH 1/2] power: supply: max17042_battery: use VF SOC register for capacity property
The capacity property uses the RepSOC register to report the current state of charge. This register did not provide a reliable SOC value during my testing with the max17047 variant on a Galaxy S3 (Trats2/GT-I9300). The reported value did not change or even stayed zero in some cases. However, the VF SOC register provided an accurate SOC value at all times. It uses the voltage fuel gauge to determine the SOC. Signed-off-by: Wolfgang Wiedmeyer --- drivers/power/max17042_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index da7a75f..20cb1fd 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -246,7 +246,7 @@ static int max17042_get_property(struct power_supply *psy, val->intval = data * 625 / 8; break; case POWER_SUPPLY_PROP_CAPACITY: - ret = regmap_read(map, MAX17042_RepSOC, ); + ret = regmap_read(map, MAX17042_VFSOC, ); if (ret < 0) return ret; -- Website: https://fossencdi.org OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4 Key download: https://wiedmeyer.de/keys/ww.asc
Re: [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
Krzysztof Kozlowski writes:
> On Thu, Sep 22, 2016 at 08:59:03PM +0200, Wolfgang Wiedmeyer wrote:
>>
>> Krzysztof Kozlowski writes:
>>
>> > On Thu, Sep 22, 2016 at 06:48:35PM +0200, Wolfgang Wiedmeyer wrote:
>> >> This allows to reboot the device into recovery mode and into the download
>> >> mode of the bootloader.
>> >
>> > Which bootloader? Probably UBoot... or Samsung stock one? Could you put
>> > that information here?
>>
>> I'm only working with the stock one. I was under the impression that the
>> stock bootloader cannot be replaced on a i9300 because there's a
>> signature check. Is UBoot loaded after the stock one on Trats2 or how
>> does this work? I didn't find information on that.
>
> +CC Marek,
>
> Trats2 is working with U-Boot. Just U-Boot. However I never converted S3
> into Trats2 on my own. I always used targets prepared to be "Trats2"
> type.
It would be awesome to be able to run U-Boot on i9300. Is there a way to
test this by not risking to brick the device, e.g. by booting from the
SD card? Then I could send an updated version of the patch that is
compatible with U-Boot :)
> Of course kernel is independent to bootloader but in that case you want
> to use a specific interface between kernel and specific bootloader
> type/version. In that case - this should be U-Boot, I think.
I absolutely understand that. Having the patch in mainline doesn't make
much sense if it's not compatible with U-Boot.
Best regards,
Wolfgang
>> >> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>> >> ---
>> >> arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
>> >> arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
>> >> 2 files changed, 15 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> b/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> index 129e973..a38d1e3 100644
>> >> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> @@ -1294,3 +1294,17 @@
>> >> vtmu-supply = <_reg>;
>> >> status = "okay";
>> >> };
>> >> +
>> >> + {
>> >> + compatible = "syscon", "simple-mfd";
>> >> +
>> >> + reboot-mode {
>> >> + compatible = "syscon-reboot-mode";
>> >> + offset = <0x80c>;
>> >> +
>> >> + mode-normal = <0x12345670>;
>> >> + mode-bootloader = <0x12345671>;
>> >> + mode-download = <0x12345671>;
>> >> + mode-recovery = <0x12345674>;
>> >
>> > Hmmm, how did you get these values? Are they already supported?
>>
>> I only have the vendor source drop as documentation. The magic mode
>> values [1] and the offset [2] can be found there.
>
> It would be useful to mention that in commit msg (just the source)...
> however as I wrote above, these values should be for U-Boot, not the
> stock one.
>
> Best regards,
> Krzysztof
>
>>
>> > It would be nice to document them:
>> > 1. In Documentation/arm/Samsung/Bootloader-interface.txt
>> > 2. In header. I hate such magic numbers... you could add new header next
>> >to existing rockchip one:
>> >include/dt-bindings/soc/samsung,boot-mode.h
>> >(and update maintainers entry :) )
>>
>> Thanks for the review! I will do the documentation and update the commit
>> message.
>>
>> Best regards,
>> Wolfgang
>>
>> [1]
>> https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/sec-reboot.c#n65
>>
>> [2]
>> https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/include/mach/regs-pmu.h#n79
>>
>>
>> --
>> Website: https://fossencdi.org
>> Jabber: wolfg...@wiedmeyer.de
>> OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
>> Key download: https://wiedmeyer.de/keys/ww.asc
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
Krzysztof Kozlowski writes:
> On Thu, Sep 22, 2016 at 08:59:03PM +0200, Wolfgang Wiedmeyer wrote:
>>
>> Krzysztof Kozlowski writes:
>>
>> > On Thu, Sep 22, 2016 at 06:48:35PM +0200, Wolfgang Wiedmeyer wrote:
>> >> This allows to reboot the device into recovery mode and into the download
>> >> mode of the bootloader.
>> >
>> > Which bootloader? Probably UBoot... or Samsung stock one? Could you put
>> > that information here?
>>
>> I'm only working with the stock one. I was under the impression that the
>> stock bootloader cannot be replaced on a i9300 because there's a
>> signature check. Is UBoot loaded after the stock one on Trats2 or how
>> does this work? I didn't find information on that.
>
> +CC Marek,
>
> Trats2 is working with U-Boot. Just U-Boot. However I never converted S3
> into Trats2 on my own. I always used targets prepared to be "Trats2"
> type.
It would be awesome to be able to run U-Boot on i9300. Is there a way to
test this by not risking to brick the device, e.g. by booting from the
SD card? Then I could send an updated version of the patch that is
compatible with U-Boot :)
> Of course kernel is independent to bootloader but in that case you want
> to use a specific interface between kernel and specific bootloader
> type/version. In that case - this should be U-Boot, I think.
I absolutely understand that. Having the patch in mainline doesn't make
much sense if it's not compatible with U-Boot.
Best regards,
Wolfgang
>> >> Signed-off-by: Wolfgang Wiedmeyer
>> >> ---
>> >> arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
>> >> arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
>> >> 2 files changed, 15 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> b/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> index 129e973..a38d1e3 100644
>> >> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
>> >> @@ -1294,3 +1294,17 @@
>> >> vtmu-supply = <_reg>;
>> >> status = "okay";
>> >> };
>> >> +
>> >> + {
>> >> + compatible = "syscon", "simple-mfd";
>> >> +
>> >> + reboot-mode {
>> >> + compatible = "syscon-reboot-mode";
>> >> + offset = <0x80c>;
>> >> +
>> >> + mode-normal = <0x12345670>;
>> >> + mode-bootloader = <0x12345671>;
>> >> + mode-download = <0x12345671>;
>> >> + mode-recovery = <0x12345674>;
>> >
>> > Hmmm, how did you get these values? Are they already supported?
>>
>> I only have the vendor source drop as documentation. The magic mode
>> values [1] and the offset [2] can be found there.
>
> It would be useful to mention that in commit msg (just the source)...
> however as I wrote above, these values should be for U-Boot, not the
> stock one.
>
> Best regards,
> Krzysztof
>
>>
>> > It would be nice to document them:
>> > 1. In Documentation/arm/Samsung/Bootloader-interface.txt
>> > 2. In header. I hate such magic numbers... you could add new header next
>> >to existing rockchip one:
>> >include/dt-bindings/soc/samsung,boot-mode.h
>> >(and update maintainers entry :) )
>>
>> Thanks for the review! I will do the documentation and update the commit
>> message.
>>
>> Best regards,
>> Wolfgang
>>
>> [1]
>> https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/sec-reboot.c#n65
>>
>> [2]
>> https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/include/mach/regs-pmu.h#n79
>>
>>
>> --
>> Website: https://fossencdi.org
>> Jabber: wolfg...@wiedmeyer.de
>> OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
>> Key download: https://wiedmeyer.de/keys/ww.asc
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
Krzysztof Kozlowski writes:
> On Thu, Sep 22, 2016 at 06:48:35PM +0200, Wolfgang Wiedmeyer wrote:
>> This allows to reboot the device into recovery mode and into the download
>> mode of the bootloader.
>
> Which bootloader? Probably UBoot... or Samsung stock one? Could you put
> that information here?
I'm only working with the stock one. I was under the impression that the
stock bootloader cannot be replaced on a i9300 because there's a
signature check. Is UBoot loaded after the stock one on Trats2 or how
does this work? I didn't find information on that.
>>
>> Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
>> ---
>> arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
>> arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
>> b/arch/arm/boot/dts/exynos4412-trats2.dts
>> index 129e973..a38d1e3 100644
>> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
>> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
>> @@ -1294,3 +1294,17 @@
>> vtmu-supply = <_reg>;
>> status = "okay";
>> };
>> +
>> + {
>> +compatible = "syscon", "simple-mfd";
>> +
>> +reboot-mode {
>> +compatible = "syscon-reboot-mode";
>> +offset = <0x80c>;
>> +
>> +mode-normal = <0x12345670>;
>> +mode-bootloader = <0x12345671>;
>> +mode-download = <0x12345671>;
>> +mode-recovery = <0x12345674>;
>
> Hmmm, how did you get these values? Are they already supported?
I only have the vendor source drop as documentation. The magic mode
values [1] and the offset [2] can be found there.
> It would be nice to document them:
> 1. In Documentation/arm/Samsung/Bootloader-interface.txt
> 2. In header. I hate such magic numbers... you could add new header next
>to existing rockchip one:
>include/dt-bindings/soc/samsung,boot-mode.h
>(and update maintainers entry :) )
Thanks for the review! I will do the documentation and update the commit
message.
Best regards,
Wolfgang
[1]
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/sec-reboot.c#n65
[2]
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/include/mach/regs-pmu.h#n79
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
Re: [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
Krzysztof Kozlowski writes:
> On Thu, Sep 22, 2016 at 06:48:35PM +0200, Wolfgang Wiedmeyer wrote:
>> This allows to reboot the device into recovery mode and into the download
>> mode of the bootloader.
>
> Which bootloader? Probably UBoot... or Samsung stock one? Could you put
> that information here?
I'm only working with the stock one. I was under the impression that the
stock bootloader cannot be replaced on a i9300 because there's a
signature check. Is UBoot loaded after the stock one on Trats2 or how
does this work? I didn't find information on that.
>>
>> Signed-off-by: Wolfgang Wiedmeyer
>> ---
>> arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
>> arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
>> b/arch/arm/boot/dts/exynos4412-trats2.dts
>> index 129e973..a38d1e3 100644
>> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
>> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
>> @@ -1294,3 +1294,17 @@
>> vtmu-supply = <_reg>;
>> status = "okay";
>> };
>> +
>> + {
>> +compatible = "syscon", "simple-mfd";
>> +
>> +reboot-mode {
>> +compatible = "syscon-reboot-mode";
>> +offset = <0x80c>;
>> +
>> +mode-normal = <0x12345670>;
>> +mode-bootloader = <0x12345671>;
>> +mode-download = <0x12345671>;
>> +mode-recovery = <0x12345674>;
>
> Hmmm, how did you get these values? Are they already supported?
I only have the vendor source drop as documentation. The magic mode
values [1] and the offset [2] can be found there.
> It would be nice to document them:
> 1. In Documentation/arm/Samsung/Bootloader-interface.txt
> 2. In header. I hate such magic numbers... you could add new header next
>to existing rockchip one:
>include/dt-bindings/soc/samsung,boot-mode.h
>(and update maintainers entry :) )
Thanks for the review! I will do the documentation and update the commit
message.
Best regards,
Wolfgang
[1]
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/sec-reboot.c#n65
[2]
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/arch/arm/mach-exynos/include/mach/regs-pmu.h#n79
--
Website: https://fossencdi.org
Jabber: wolfg...@wiedmeyer.de
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
signature.asc
Description: PGP signature
[PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
This allows to reboot the device into recovery mode and into the download
mode of the bootloader.
Signed-off-by: Wolfgang Wiedmeyer <wolf...@wiedmeyer.de>
---
arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 129e973..a38d1e3 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -1294,3 +1294,17 @@
vtmu-supply = <_reg>;
status = "okay";
};
+
+ {
+ compatible = "syscon", "simple-mfd";
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x80c>;
+
+ mode-normal = <0x12345670>;
+ mode-bootloader = <0x12345671>;
+ mode-download = <0x12345671>;
+ mode-recovery = <0x12345674>;
+ };
+};
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi
b/arch/arm/boot/dts/exynos4x12.dtsi
index c452499..b58c14e 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -179,7 +179,7 @@
ranges;
status = "disabled";
- pmu@1002 {
+ pmu: pmu@1002 {
reg = <0x1002 0x3000>;
};
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
[PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
This allows to reboot the device into recovery mode and into the download
mode of the bootloader.
Signed-off-by: Wolfgang Wiedmeyer
---
arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++
arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 129e973..a38d1e3 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -1294,3 +1294,17 @@
vtmu-supply = <_reg>;
status = "okay";
};
+
+ {
+ compatible = "syscon", "simple-mfd";
+
+ reboot-mode {
+ compatible = "syscon-reboot-mode";
+ offset = <0x80c>;
+
+ mode-normal = <0x12345670>;
+ mode-bootloader = <0x12345671>;
+ mode-download = <0x12345671>;
+ mode-recovery = <0x12345674>;
+ };
+};
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi
b/arch/arm/boot/dts/exynos4x12.dtsi
index c452499..b58c14e 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -179,7 +179,7 @@
ranges;
status = "disabled";
- pmu@1002 {
+ pmu: pmu@1002 {
reg = <0x1002 0x3000>;
};
--
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE 048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc
