subject:"\[U\-Boot\] \[PATCH 08\/16\] pmic\:muic\: Support for MUIC built into MAX8997 device"

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

2012-10-02 Thread Lukasz Majewski

Hi Stefano,

 On 14/09/2012 17:40, Lukasz Majewski wrote:
  Support for MUIC (Micro USB Integrated Circuit) built into the
  MAX8997 power management device.
  
  The MUIC device will work with redesigned PMIC framework.
  
  Signed-off-by: Lukasz Majewski l.majew...@samsung.com
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  Cc: Stefano Babic sba...@denx.de
  ---
 
 Hi Lukasz,
 
   drivers/misc/Makefile|1 +
   drivers/misc/muic_max8997.c  |   80
  ++
  include/power/max8997_muic.h |   61
   include/power/power_chrg.h   |
  1 + 4 files changed, 143 insertions(+), 0 deletions(-) create mode
  100644 drivers/misc/muic_max8997.c create mode 100644
  include/power/max8997_muic.h
  
 
 I see now with this patch, but we have a mix between drivers/misc and
 drivers/power. I think this was done because the power directory was
 introduced later, but really all pmics under drivers/misc should be
 moved into drivers/power.
 
 Maybe belong this one also to drivers/power ?
 
 You added several add-ons for MAX8997. Maybe it is clearer if
 everything goes into a separate directory, such as power/max8997/
 
  diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
  index 271463c..f08a800 100644
  --- a/drivers/misc/Makefile
  +++ b/drivers/misc/Makefile
  @@ -41,6 +41,7 @@ COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
   COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
   COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
   COBJS-$(CONFIG_PMIC_MAX8997) += pmic_max8997.o
  +COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
   
   COBJS  := $(COBJS-y)
   SRCS   := $(COBJS:.o=.c)
  diff --git a/drivers/misc/muic_max8997.c
  b/drivers/misc/muic_max8997.c new file mode 100644
  index 000..d332c09
  --- /dev/null
  +++ b/drivers/misc/muic_max8997.c
  @@ -0,0 +1,80 @@
  +/*
  + *  Copyright (C) 2012 Samsung Electronics
  + *  Lukasz Majewski l.majew...@samsung.com
  + *
  + * See file CREDITS for list of people who contributed to this
  + * project.
  + *
  + * This program is free software; you can redistribute it and/or
  + * modify it under the terms of the GNU General Public License as
  + * published by the Free Software Foundation; either version 2 of
  + * the License, or (at your option) any later version.
  + *
  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  + * GNU General Public License for more details.
  + *
  + * You should have received a copy of the GNU General Public
  License
  + * along with this program; if not, write to the Free Software
  + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
  + * MA 02111-1307 USA
  + */
  +
  +#include common.h
  +#include power/pmic.h
  +#include power/power_chrg.h
  +#include power/max8997_muic.h
  +#include i2c.h
  +
  +int power_muic_init(unsigned int bus)
 
 You add a different function to initialize it, but this is a version
 opf pmic_init(). Why cannot we consider this one as all other pmics,
 avoiding to introduce new and undocumented functions ?
 
 If I do not misunderstand, you use the different functions to select
 the pmic. In other words, calling power_update_battery() in the trats
 code selects automatically the fulel-gauge, because this code
 implements this function. But this is against the goal of your
 patches, that is to add multi instances of the pmics (and maybe a
 second instance of fuel_gauge..).
 
 I think this should be solved moving this function to the general API
 and calling it via pointer. For example, something like:
 
   p = pmic_get(MAX_8997);
   p-read(...);
   
   p = pmic_get(MAX17042_FG);
   p-power_update_battery(..);
 
 What do you think ?

I think that struct pmic shall be extended by another field:

struct pmic {
struct power_ops *p_ops;
};

struct bat_ops {
int (*check_battery) (struct pmic *p);
int (*update_battery) (struct pmic *p);
int (*init_battery) (struct pmic *p);   

[other battery related callbacks]
};

Each instance of struct bat_ops is tied with power related device.

I will post v2 of the framework very soon.


-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

2012-09-17 Thread Stefano Babic

On 14/09/2012 17:40, Lukasz Majewski wrote:
 Support for MUIC (Micro USB Integrated Circuit) built into the MAX8997
 power management device.
 
 The MUIC device will work with redesigned PMIC framework.
 
 Signed-off-by: Lukasz Majewski l.majew...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Stefano Babic sba...@denx.de
 ---

Hi Lukasz,

  drivers/misc/Makefile|1 +
  drivers/misc/muic_max8997.c  |   80 
 ++
  include/power/max8997_muic.h |   61 
  include/power/power_chrg.h   |1 +
  4 files changed, 143 insertions(+), 0 deletions(-)
  create mode 100644 drivers/misc/muic_max8997.c
  create mode 100644 include/power/max8997_muic.h
 

I see now with this patch, but we have a mix between drivers/misc and
drivers/power. I think this was done because the power directory was
introduced later, but really all pmics under drivers/misc should be
moved into drivers/power.

Maybe belong this one also to drivers/power ?

You added several add-ons for MAX8997. Maybe it is clearer if
everything goes into a separate directory, such as power/max8997/

 diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
 index 271463c..f08a800 100644
 --- a/drivers/misc/Makefile
 +++ b/drivers/misc/Makefile
 @@ -41,6 +41,7 @@ COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
  COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
  COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
  COBJS-$(CONFIG_PMIC_MAX8997) += pmic_max8997.o
 +COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
  
  COBJS:= $(COBJS-y)
  SRCS := $(COBJS:.o=.c)
 diff --git a/drivers/misc/muic_max8997.c b/drivers/misc/muic_max8997.c
 new file mode 100644
 index 000..d332c09
 --- /dev/null
 +++ b/drivers/misc/muic_max8997.c
 @@ -0,0 +1,80 @@
 +/*
 + *  Copyright (C) 2012 Samsung Electronics
 + *  Lukasz Majewski l.majew...@samsung.com
 + *
 + * See file CREDITS for list of people who contributed to this
 + * project.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of
 + * the License, or (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
 + * MA 02111-1307 USA
 + */
 +
 +#include common.h
 +#include power/pmic.h
 +#include power/power_chrg.h
 +#include power/max8997_muic.h
 +#include i2c.h
 +
 +int power_muic_init(unsigned int bus)

You add a different function to initialize it, but this is a version opf
pmic_init(). Why cannot we consider this one as all other pmics,
avoiding to introduce new and undocumented functions ?

If I do not misunderstand, you use the different functions to select the
pmic. In other words, calling power_update_battery() in the trats code
selects automatically the fulel-gauge, because this code implements this
function. But this is against the goal of your patches, that is to add
multi instances of the pmics (and maybe a second instance of fuel_gauge..).

I think this should be solved moving this function to the general API
and calling it via pointer. For example, something like:

p = pmic_get(MAX_8997);
p-read(...);

p = pmic_get(MAX17042_FG);
p-power_update_battery(..);

What do you think ?

Best regards,
Stefano Babic


-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

2012-09-17 Thread Lukasz Majewski

Hi Stefano,

 On 14/09/2012 17:40, Lukasz Majewski wrote:
  Support for MUIC (Micro USB Integrated Circuit) built into the
  MAX8997 power management device.
  
  The MUIC device will work with redesigned PMIC framework.
  
  Signed-off-by: Lukasz Majewski l.majew...@samsung.com
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  Cc: Stefano Babic sba...@denx.de
  ---
 
 Hi Lukasz,
 
   drivers/misc/Makefile|1 +
   drivers/misc/muic_max8997.c  |   80
  ++
  include/power/max8997_muic.h |   61
   include/power/power_chrg.h   |
  1 + 4 files changed, 143 insertions(+), 0 deletions(-) create mode
  100644 drivers/misc/muic_max8997.c create mode 100644
  include/power/max8997_muic.h
  
 
 I see now with this patch, but we have a mix between drivers/misc and
 drivers/power. I think this was done because the power directory was
 introduced later, but really all pmics under drivers/misc should be
 moved into drivers/power.

Yes, I agree about that. PMIC (or in the future POWER) related code
shall be moved to drivers/power/{subdir_per_PMIC_device}

Please also keep in mind, that I plan to rename the pmic
command/framework to power to better reflect its purpose.

 
 Maybe belong this one also to drivers/power ?
 
 You added several add-ons for MAX8997. Maybe it is clearer if
 everything goes into a separate directory, such as power/max8997/

Very good idea.
I've already proposed a little cleanup with putting all power related
includes to ./include/power/ directory.

 
  diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
  index 271463c..f08a800 100644
  --- a/drivers/misc/Makefile
  +++ b/drivers/misc/Makefile
  @@ -41,6 +41,7 @@ COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
   COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
   COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
   COBJS-$(CONFIG_PMIC_MAX8997) += pmic_max8997.o
  +COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
   
   COBJS  := $(COBJS-y)
   SRCS   := $(COBJS:.o=.c)
  diff --git a/drivers/misc/muic_max8997.c
  b/drivers/misc/muic_max8997.c new file mode 100644
  index 000..d332c09
  --- /dev/null
  +++ b/drivers/misc/muic_max8997.c
  @@ -0,0 +1,80 @@
  +/*
  + *  Copyright (C) 2012 Samsung Electronics
  + *  Lukasz Majewski l.majew...@samsung.com
  + *
  + * See file CREDITS for list of people who contributed to this
  + * project.
  + *
  + * This program is free software; you can redistribute it and/or
  + * modify it under the terms of the GNU General Public License as
  + * published by the Free Software Foundation; either version 2 of
  + * the License, or (at your option) any later version.
  + *
  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  + * GNU General Public License for more details.
  + *
  + * You should have received a copy of the GNU General Public
  License
  + * along with this program; if not, write to the Free Software
  + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
  + * MA 02111-1307 USA
  + */
  +
  +#include common.h
  +#include power/pmic.h
  +#include power/power_chrg.h
  +#include power/max8997_muic.h
  +#include i2c.h
  +
  +int power_muic_init(unsigned int bus)
 
 You add a different function to initialize it, but this is a version
 opf pmic_init(). Why cannot we consider this one as all other pmics,
 avoiding to introduce new and undocumented functions ?
 
 If I do not misunderstand, you use the different functions to select
 the pmic. In other words, calling power_update_battery() in the trats
 code selects automatically the fulel-gauge, because this code
 implements this function. But this is against the goal of your
 patches, that is to add multi instances of the pmics (and maybe a
 second instance of fuel_gauge..).
 
 I think this should be solved moving this function to the general API
 and calling it via pointer. For example, something like:
 
   p = pmic_get(MAX_8997);
   p-read(...);
   
   p = pmic_get(MAX17042_FG);
   p-power_update_battery(..);
 
 What do you think ?

I will think about that and give you an answer.
 
 Best regards,
 Stefano Babic
 
 

Regards,
Lukasz

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

2012-09-14 Thread Lukasz Majewski

Support for MUIC (Micro USB Integrated Circuit) built into the MAX8997
power management device.

The MUIC device will work with redesigned PMIC framework.

Signed-off-by: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Stefano Babic sba...@denx.de
---
 drivers/misc/Makefile|1 +
 drivers/misc/muic_max8997.c  |   80 ++
 include/power/max8997_muic.h |   61 
 include/power/power_chrg.h   |1 +
 4 files changed, 143 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/muic_max8997.c
 create mode 100644 include/power/max8997_muic.h

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 271463c..f08a800 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -41,6 +41,7 @@ COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
 COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
 COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
 COBJS-$(CONFIG_PMIC_MAX8997) += pmic_max8997.o
+COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
 
 COBJS  := $(COBJS-y)
 SRCS   := $(COBJS:.o=.c)
diff --git a/drivers/misc/muic_max8997.c b/drivers/misc/muic_max8997.c
new file mode 100644
index 000..d332c09
--- /dev/null
+++ b/drivers/misc/muic_max8997.c
@@ -0,0 +1,80 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics
+ *  Lukasz Majewski l.majew...@samsung.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include common.h
+#include power/pmic.h
+#include power/power_chrg.h
+#include power/max8997_muic.h
+#include i2c.h
+
+int power_muic_init(unsigned int bus)
+{
+   struct pmic *p = pmic_alloc();
+   static const char name[] = MAX8997_MUIC;
+
+   puts(Board Micro USB Interface Controller init\n);
+
+   p-name = name;
+   p-interface = PMIC_I2C;
+   p-number_of_regs = MUIC_NUM_OF_REGS;
+   p-hw.i2c.addr = MAX8997_MUIC_I2C_ADDR;
+   p-hw.i2c.tx_num = 1;
+   p-bus = bus;
+
+   return 0;
+}
+
+int power_chrg_get_type(void)
+{
+   unsigned int val;
+   unsigned char charge_type, charger;
+   struct pmic *p = pmic_get(MAX8997_MUIC);
+
+   if (pmic_probe(p))
+   return CHARGER_NO;
+
+   pmic_reg_read(p, MAX8997_MUIC_STATUS2, val);
+   charge_type = val  MAX8997_MUIC_CHG_MASK;
+
+   switch (charge_type) {
+   case MAX8997_MUIC_CHG_NO:
+   charger = CHARGER_NO;
+   break;
+   case MAX8997_MUIC_CHG_USB:
+   case MAX8997_MUIC_CHG_USB_D:
+   charger = CHARGER_USB;
+   break;
+   case MAX8997_MUIC_CHG_TA:
+   case MAX8997_MUIC_CHG_TA_1A:
+   charger = CHARGER_TA;
+   break;
+   case MAX8997_MUIC_CHG_TA_500:
+   charger = CHARGER_TA_500;
+   break;
+   default:
+   charger = CHARGER_UNKNOWN;
+   break;
+   }
+
+   return charger;
+}
diff --git a/include/power/max8997_muic.h b/include/power/max8997_muic.h
new file mode 100644
index 000..0149c12
--- /dev/null
+++ b/include/power/max8997_muic.h
@@ -0,0 +1,61 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics
+ *  Lukasz Majewski l.majew...@samsung.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __MAX8997_MUIC_H_
+#define __MAX8997_MUIC_H_
+
+#include power/power_chrg.h
+
+/* MAX8997_MUIC_STATUS2 */
+#define MAX8997_MUIC_CHG_NO0x00
+#define MAX8997_MUIC_CHG_USB   0x01
+#define

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

Re: [U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

[U-Boot] [PATCH 08/16] pmic:muic: Support for MUIC built into MAX8997 device

4 matches

Site Navigation

Mail list logo

Footer information