Hello Suriyan,

On 10/24/2014 05:53 PM, Suriyan Ramasami wrote:
Hello Jaehoon Chung,


On Thu, Oct 23, 2014 at 11:52 PM, Jaehoon Chung <jh80.ch...@samsung.com> wrote:
Hi.

On 10/21/2014 02:52 AM, Suriyan Ramasami wrote:
Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.

Signed-off-by: Suriyan Ramasami <suriya...@gmail.com>
---
  drivers/power/pmic/pmic_max77686.c | 48 +++++++++++++++++++++++++++++++++++++-
  include/power/max77686_pmic.h      |  3 +++
  2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/power/pmic/pmic_max77686.c 
b/drivers/power/pmic/pmic_max77686.c
index df1fd91..1aeadb4 100644
--- a/drivers/power/pmic/pmic_max77686.c
+++ b/drivers/power/pmic/pmic_max77686.c
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV)
       return 0;
  }

+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+     unsigned int hex = 0;
+
+     if (buck < 5 || buck > 9) {
+             debug("%s: buck %d is not supported\n", __func__, buck);
+             return 0;

Here, I should return MAX77686_BUCK_VOLT_MAX_HEX + 1 instead of 0 to
signal an error condition, as 0 is a valid non error value.

He 'hex' value has at most 1 byte of len, so you can return the 'int' value and use the negative errno numbers - and there is no value conflicts.

+     }
+
+     hex = (uV - 750000) / 50000;
+
+     if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+             return hex;
+
+     debug("%s: %ld is wrong voltage value for BUCK%d\n",
+           __func__, uV, buck);
+     return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
  {
       unsigned int val, ret, hex, adr;
@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong 
uV)
       return ret;
  }

+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+     unsigned int val, ret, hex, adr;
+
+     if (buck < 5 && buck > 9) {
+             printf("%s: %d is an unsupported bucket number\n",
+                    __func__, buck);
+             return -1;

How about using error number instead of "-1"?

Could you please elaborate on this? This function always returns -1 on
failure just like the function max77686_set_ldo_voltate() which I have
tried to mimic as much as I can.
I am guessing that you want me to return -EINVAL in this case? Please
let me know, and I am OK to change it, just that this function will
deviate from the rest of the functions in this file.

Yes, this will be better.

+     }
+
+     adr = max77686_buck_addr[buck] + 1;
+     hex = max77686_buck_volt2hex(buck, uV);
+

if (hex < 0)
        return hex;

+     if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+             return -1;

Ditto.

I believe, I return -EINVAL here, but please look at my reasoning above.


+
+     ret = pmic_reg_read(p, adr, &val);
+     if (ret)
+             return ret;
+
+     val &= ~MAX77686_BUCK_VOLT_MASK;
+     val |= hex;
+     ret |= pmic_reg_write(p, adr, val);

ret |= pmic_reg_write(p, addr, val | hex); ?


OK, will change that. Again, this was done to be as close to
max77686_set_ldo_voltate()

Thanks and Regards!
- Suriyan

Best Regards,
Jaehoon Chung
+
+     return ret;
+}
+
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
  {
       unsigned int val, ret, adr, mode;
@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char 
opmode)
       /* mode */
       switch (opmode) {
       case OPMODE_OFF:
-             mode = MAX77686_BUCK_MODE_OFF;
+             mode = MAX77686_BUCK_MODE_OFF << mode_shift;
               break;
       case OPMODE_STANDBY:
               switch (buck) {
diff --git a/include/power/max77686_pmic.h b/include/power/max77686_pmic.h
index c2a772a..b0e4255 100644
--- a/include/power/max77686_pmic.h
+++ b/include/power/max77686_pmic.h
@@ -150,6 +150,7 @@ enum {

  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
  int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);

  #define MAX77686_LDO_VOLT_MAX_HEX    0x3f
@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char 
opmode);
  #define MAX77686_LDO_MODE_STANDBY    (0x01 << 0x06)
  #define MAX77686_LDO_MODE_LPM                (0x02 << 0x06)
  #define MAX77686_LDO_MODE_ON         (0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX   0x3f
+#define MAX77686_BUCK_VOLT_MASK              0x3f
  #define MAX77686_BUCK_MODE_MASK              0x03
  #define MAX77686_BUCK_MODE_SHIFT_1   0x00
  #define MAX77686_BUCK_MODE_SHIFT_2   0x04



Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to