* [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver
2013-07-19 19:00 [U-Boot] [PATCH 1/6] spl/Makefile: Add drivers/power/pmic/libpmic to CONFIG_SPL_POWER_SUPPORT Tom Rini
@ 2013-07-19 19:00 ` Tom Rini
2013-07-23 18:34 ` Dan Murphy
2013-07-19 19:00 ` [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver Tom Rini
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:00 UTC (permalink / raw)
To: u-boot
From: Greg Guyotte <gguyotte@ti.com>
Add a driver for the TPS65217 PMIC that is found in the Beaglebone
family of boards.
Signed-off-by: Greg Guyotte <gguyotte@ti.com>
[trini: Split and rework Greg's changes into new drivers/power
framework]
Signed-off-by: Tom Rini <trini@ti.com>
---
drivers/power/pmic/Makefile | 1 +
drivers/power/pmic/pmic_tps65217.c | 108 ++++++++++++++++++++++++++++++++++++
include/power/tps65217.h | 92 ++++++++++++++++++++++++++++++
3 files changed, 201 insertions(+)
create mode 100644 drivers/power/pmic/pmic_tps65217.c
create mode 100644 include/power/tps65217.h
diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
index 14d426f..473cb80 100644
--- a/drivers/power/pmic/Makefile
+++ b/drivers/power/pmic/Makefile
@@ -29,6 +29,7 @@ COBJS-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
+COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
COBJS := $(COBJS-y)
SRCS := $(COBJS:.o=.c)
diff --git a/drivers/power/pmic/pmic_tps65217.c b/drivers/power/pmic/pmic_tps65217.c
new file mode 100644
index 0000000..c84bbcd
--- /dev/null
+++ b/drivers/power/pmic/pmic_tps65217.c
@@ -0,0 +1,108 @@
+/*
+ * (C) Copyright 2011-2013
+ * Texas Instruments, <www.ti.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 <i2c.h>
+#include <power/tps65217.h>
+
+/**
+ * tps65217_reg_read() - Generic function that can read a TPS65217 register
+ * @src_reg: Source register address
+ * @src_val: Address of destination variable
+ */
+uchar tps65217_reg_read(uchar src_reg, uchar *src_val)
+{
+ if (i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1))
+ return 1;
+ return 0;
+}
+
+/**
+ * tps65217_reg_write() - Generic function that can write a TPS65217 PMIC
+ * register or bit field regardless of protection
+ * level.
+ *
+ * @prot_level: Register password protection.
+ * use PROT_LEVEL_NONE, PROT_LEVEL_1, or PROT_LEVEL_2
+ * @dest_reg: Register address to write.
+ * @dest_val: Value to write.
+ * @mask: Bit mask (8 bits) to be applied. Function will only
+ * change bits that are set in the bit mask.
+ *
+ * @return: 0 for success, 1 for failure.
+ */
+int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
+ uchar mask)
+{
+ uchar read_val;
+ uchar xor_reg;
+
+ /*
+ * If we are affecting only a bit field, read dest_reg and apply the
+ * mask
+ */
+ if (mask != MASK_ALL_BITS) {
+ if (i2c_read(TPS65217_CHIP_PM, dest_reg, 1, &read_val, 1))
+ return 1;
+ read_val &= (~mask);
+ read_val |= (dest_val & mask);
+ dest_val = read_val;
+ }
+
+ if (prot_level > 0) {
+ xor_reg = dest_reg ^ PASSWORD_UNLOCK;
+ if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
+ return 1;
+ }
+
+ if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
+ return 1;
+
+ if (prot_level == PROT_LEVEL_2) {
+ if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
+ return 1;
+
+ if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
+ return 1;
+ }
+
+ return 0;
+}
+
+int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)
+{
+ if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
+ (dc_cntrl_reg != DEFDCDC3))
+ return 1;
+
+ /* set voltage level */
+ if (tps65217_reg_write(PROT_LEVEL_2, dc_cntrl_reg, volt_sel,
+ MASK_ALL_BITS))
+ return 1;
+
+ /* set GO bit to initiate voltage transition */
+ if (tps65217_reg_write(PROT_LEVEL_2, DEFSLEW, DCDC_GO, DCDC_GO))
+ return 1;
+
+ return 0;
+}
diff --git a/include/power/tps65217.h b/include/power/tps65217.h
new file mode 100644
index 0000000..c12a709
--- /dev/null
+++ b/include/power/tps65217.h
@@ -0,0 +1,92 @@
+/*
+ * (C) Copyright 2011-2013
+ * Texas Instruments, <www.ti.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 __TPS65217_H__
+#define __TPS65217_H__
+
+/* I2C chip address */
+#define TPS65217_CHIP_PM 0x24
+
+/* Registers */
+#define CHIPID 0x00
+#define POWER_PATH 0x01
+#define INTERRUPT 0x02
+#define CHGCONFIG0 0x03
+#define CHGCONFIG1 0x04
+#define CHGCONFIG2 0x05
+#define CHGCONFIG3 0x06
+#define WLEDCTRL1 0x07
+#define WLEDCTRL2 0x08
+#define MUXCTRL 0x09
+#define STATUS 0x0A
+#define PASSWORD 0x0B
+#define PGOOD 0x0C
+#define DEFPG 0x0D
+#define DEFDCDC1 0x0E
+#define DEFDCDC2 0x0F
+#define DEFDCDC3 0x10
+#define DEFSLEW 0x11
+#define DEFLDO1 0x12
+#define DEFLDO2 0x13
+#define DEFLS1 0x14
+#define DEFLS2 0x15
+#define ENABLE 0x16
+#define DEFUVLO 0x18
+#define SEQ1 0x19
+#define SEQ2 0x1A
+#define SEQ3 0x1B
+#define SEQ4 0x1C
+#define SEQ5 0x1D
+#define SEQ6 0x1E
+
+#define PROT_LEVEL_NONE 0x00
+#define PROT_LEVEL_1 0x01
+#define PROT_LEVEL_2 0x02
+
+#define PASSWORD_LOCK_FOR_WRITE 0x00
+#define PASSWORD_UNLOCK 0x7D
+
+#define DCDC_GO 0x80
+
+#define MASK_ALL_BITS 0xFF
+
+#define USB_INPUT_CUR_LIMIT_MASK 0x03
+#define USB_INPUT_CUR_LIMIT_100MA 0x00
+#define USB_INPUT_CUR_LIMIT_500MA 0x01
+#define USB_INPUT_CUR_LIMIT_1300MA 0x02
+#define USB_INPUT_CUR_LIMIT_1800MA 0x03
+
+#define DCDC_VOLT_SEL_1275MV 0x0F
+#define DCDC_VOLT_SEL_1325MV 0x11
+
+#define LDO_MASK 0x1F
+#define LDO_VOLTAGE_OUT_3_3 0x1F
+
+#define PWR_SRC_USB_BITMASK 0x4
+#define PWR_SRC_AC_BITMASK 0x8
+
+uchar tps65217_reg_read(uchar src_reg, uchar *src_val);
+int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
+ uchar mask);
+int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel);
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver
2013-07-19 19:00 ` [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver Tom Rini
@ 2013-07-23 18:34 ` Dan Murphy
2013-07-25 14:37 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2013-07-23 18:34 UTC (permalink / raw)
To: u-boot
On 07/19/2013 02:00 PM, Tom Rini wrote:
> From: Greg Guyotte <gguyotte@ti.com>
>
> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> family of boards.
>
> Signed-off-by: Greg Guyotte <gguyotte@ti.com>
> [trini: Split and rework Greg's changes into new drivers/power
> framework]
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/pmic_tps65217.c | 108 ++++++++++++++++++++++++++++++++++++
> include/power/tps65217.h | 92 ++++++++++++++++++++++++++++++
> 3 files changed, 201 insertions(+)
> create mode 100644 drivers/power/pmic/pmic_tps65217.c
> create mode 100644 include/power/tps65217.h
>
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 14d426f..473cb80 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
> COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
> COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
> COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
> +COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
>
> COBJS := $(COBJS-y)
> SRCS := $(COBJS:.o=.c)
> diff --git a/drivers/power/pmic/pmic_tps65217.c b/drivers/power/pmic/pmic_tps65217.c
> new file mode 100644
> index 0000000..c84bbcd
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65217.c
> @@ -0,0 +1,108 @@
> +/*
> + * (C) Copyright 2011-2013
Curious if this is the first time in why does it have a 2011 copyright?
> + * Texas Instruments, <www.ti.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 <i2c.h>
> +#include <power/tps65217.h>
> +
> +/**
> + * tps65217_reg_read() - Generic function that can read a TPS65217 register
> + * @src_reg: Source register address
> + * @src_val: Address of destination variable
No return defined here in the brief
> + */
> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val)
> +{
> + if (i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1))
> + return 1;
This may be nit picky but generally in error cases we return negative.
Also why not return an error from errno?
Also why an uchar when you are returning an int?
> + return 0;
> +}
> +
> +/**
> + * tps65217_reg_write() - Generic function that can write a TPS65217 PMIC
> + * register or bit field regardless of protection
> + * level.
> + *
> + * @prot_level: Register password protection.
> + * use PROT_LEVEL_NONE, PROT_LEVEL_1, or PROT_LEVEL_2
> + * @dest_reg: Register address to write.
> + * @dest_val: Value to write.
> + * @mask: Bit mask (8 bits) to be applied. Function will only
> + * change bits that are set in the bit mask.
> + *
> + * @return: 0 for success, 1 for failure.
> + */
> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
is prot_level a uchar or int?
Also would it not be better to have an interface that will check for mask and do the read and
just have a dedicated write function?
> + uchar mask)
> +{
> + uchar read_val;
> + uchar xor_reg;
> +
> + /*
> + * If we are affecting only a bit field, read dest_reg and apply the
> + * mask
> + */
> + if (mask != MASK_ALL_BITS) {
> + if (i2c_read(TPS65217_CHIP_PM, dest_reg, 1, &read_val, 1))
> + return 1;
> + read_val &= (~mask);
> + read_val |= (dest_val & mask);
> + dest_val = read_val;
> + }
> +
> + if (prot_level > 0) {
> + xor_reg = dest_reg ^ PASSWORD_UNLOCK;
> + if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
> + return 1;
Same comment as above
> + }
> +
> + if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
> + return 1;
> +
> + if (prot_level == PROT_LEVEL_2) {
> + if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
> + return 1;
> +
> + if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)
No header for the interface
> +{
> + if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
> + (dc_cntrl_reg != DEFDCDC3))
What do these magic numbers mean? Are these HEX numbers or a string?
> + return 1;
> +
> + /* set voltage level */
> + if (tps65217_reg_write(PROT_LEVEL_2, dc_cntrl_reg, volt_sel,
> + MASK_ALL_BITS))
> + return 1;
> +
> + /* set GO bit to initiate voltage transition */
> + if (tps65217_reg_write(PROT_LEVEL_2, DEFSLEW, DCDC_GO, DCDC_GO))
> + return 1;
> +
> + return 0;
> +}
> diff --git a/include/power/tps65217.h b/include/power/tps65217.h
> new file mode 100644
> index 0000000..c12a709
> --- /dev/null
> +++ b/include/power/tps65217.h
> @@ -0,0 +1,92 @@
> +/*
> + * (C) Copyright 2011-2013
Same copyright comment as above
> + * Texas Instruments, <www.ti.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 __TPS65217_H__
> +#define __TPS65217_H__
> +
> +/* I2C chip address */
> +#define TPS65217_CHIP_PM 0x24
> +
> +/* Registers */
> +#define CHIPID 0x00
> +#define POWER_PATH 0x01
> +#define INTERRUPT 0x02
> +#define CHGCONFIG0 0x03
> +#define CHGCONFIG1 0x04
> +#define CHGCONFIG2 0x05
> +#define CHGCONFIG3 0x06
> +#define WLEDCTRL1 0x07
> +#define WLEDCTRL2 0x08
> +#define MUXCTRL 0x09
> +#define STATUS 0x0A
> +#define PASSWORD 0x0B
> +#define PGOOD 0x0C
> +#define DEFPG 0x0D
> +#define DEFDCDC1 0x0E
> +#define DEFDCDC2 0x0F
> +#define DEFDCDC3 0x10
> +#define DEFSLEW 0x11
> +#define DEFLDO1 0x12
> +#define DEFLDO2 0x13
> +#define DEFLS1 0x14
> +#define DEFLS2 0x15
> +#define ENABLE 0x16
> +#define DEFUVLO 0x18
> +#define SEQ1 0x19
> +#define SEQ2 0x1A
> +#define SEQ3 0x1B
> +#define SEQ4 0x1C
> +#define SEQ5 0x1D
> +#define SEQ6 0x1E
> +
> +#define PROT_LEVEL_NONE 0x00
Are these registers or a mask now?
> +#define PROT_LEVEL_1 0x01
> +#define PROT_LEVEL_2 0x02
> +
> +#define PASSWORD_LOCK_FOR_WRITE 0x00
> +#define PASSWORD_UNLOCK 0x7D
> +
> +#define DCDC_GO 0x80
> +
> +#define MASK_ALL_BITS 0xFF
> +
> +#define USB_INPUT_CUR_LIMIT_MASK 0x03
> +#define USB_INPUT_CUR_LIMIT_100MA 0x00
> +#define USB_INPUT_CUR_LIMIT_500MA 0x01
> +#define USB_INPUT_CUR_LIMIT_1300MA 0x02
> +#define USB_INPUT_CUR_LIMIT_1800MA 0x03
> +
> +#define DCDC_VOLT_SEL_1275MV 0x0F
> +#define DCDC_VOLT_SEL_1325MV 0x11
> +
> +#define LDO_MASK 0x1F
> +#define LDO_VOLTAGE_OUT_3_3 0x1F
> +
> +#define PWR_SRC_USB_BITMASK 0x4
> +#define PWR_SRC_AC_BITMASK 0x8
> +
> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val);
> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
> + uchar mask);
> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel);
Are these interfaces supposed to be accessed by an outside object?
Typically there should be no direct register access from other objects.
> +#endif
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver
2013-07-23 18:34 ` Dan Murphy
@ 2013-07-25 14:37 ` Tom Rini
2013-07-25 17:21 ` Dan Murphy
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2013-07-25 14:37 UTC (permalink / raw)
To: u-boot
On Tue, Jul 23, 2013 at 01:34:15PM -0500, Dan Murphy wrote:
> On 07/19/2013 02:00 PM, Tom Rini wrote:
> > From: Greg Guyotte <gguyotte@ti.com>
> >
> > Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> > family of boards.
> >
> > Signed-off-by: Greg Guyotte <gguyotte@ti.com>
> > [trini: Split and rework Greg's changes into new drivers/power
> > framework]
> > Signed-off-by: Tom Rini <trini@ti.com>
> > ---
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/pmic_tps65217.c | 108 ++++++++++++++++++++++++++++++++++++
> > include/power/tps65217.h | 92 ++++++++++++++++++++++++++++++
> > 3 files changed, 201 insertions(+)
> > create mode 100644 drivers/power/pmic/pmic_tps65217.c
> > create mode 100644 include/power/tps65217.h
> >
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 14d426f..473cb80 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -29,6 +29,7 @@ COBJS-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
> > COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
> > COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
> > COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
> > +COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
> >
> > COBJS := $(COBJS-y)
> > SRCS := $(COBJS:.o=.c)
> > diff --git a/drivers/power/pmic/pmic_tps65217.c b/drivers/power/pmic/pmic_tps65217.c
> > new file mode 100644
> > index 0000000..c84bbcd
> > --- /dev/null
> > +++ b/drivers/power/pmic/pmic_tps65217.c
> > @@ -0,0 +1,108 @@
> > +/*
> > + * (C) Copyright 2011-2013
> Curious if this is the first time in why does it have a 2011 copyright?
Because the code was written in 2011 (and has been whacked around a few
times every year.
[snip]
> > +/**
> > + * tps65217_reg_read() - Generic function that can read a TPS65217 register
> > + * @src_reg: Source register address
> > + * @src_val: Address of destination variable
> No return defined here in the brief
Fixed.
> > + */
> > +uchar tps65217_reg_read(uchar src_reg, uchar *src_val)
> > +{
> > + if (i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1))
> > + return 1;
> This may be nit picky but generally in error cases we return negative.
> Also why not return an error from errno?
Because we're following i2c which is 0 or not 0, updated to use ret =
i2c_read(...); if (ret) return ret here and throughout.
> Also why an uchar when you are returning an int?
Fixed.
[snip]
> > +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
> is prot_level a uchar or int?
It's 0/1/2. I don't have a strong preference on if we type this out as
an int or uchar.
> Also would it not be better to have an interface that will check for
> mask and do the read and just have a dedicated write function?
I don't see the benefit, especially given the usage we have of just
updating certain bitfields at a time.
[snip]
> > +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)
> No header for the interface
Fixed.
> > +{
> > + if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
> > + (dc_cntrl_reg != DEFDCDC3))
> What do these magic numbers mean? Are these HEX numbers or a string?
OK, it took me a minute to understand your question here. These are
defines to register names, matching the TRM for the part. The register
names are however annoyingly and easily confused as hex values.
> > +#define PROT_LEVEL_NONE 0x00
> Are these registers or a mask now?
> > +#define PROT_LEVEL_1 0x01
> > +#define PROT_LEVEL_2 0x02
These are values as to what level of protection the chip has the
register under.
> > +uchar tps65217_reg_read(uchar src_reg, uchar *src_val);
> > +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
> > + uchar mask);
> > +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel);
> Are these interfaces supposed to be accessed by an outside object?
>
> Typically there should be no direct register access from other objects.
We can evaluate if there's consolidation to be done here once other
boards go and adapt MPU clock frequency scaling. What registers need to
be whacked on what board are going to decide if we can hide more
details, or not.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130725/2ece9786/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver
2013-07-25 14:37 ` Tom Rini
@ 2013-07-25 17:21 ` Dan Murphy
2013-07-25 18:05 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2013-07-25 17:21 UTC (permalink / raw)
To: u-boot
On 07/25/2013 09:37 AM, Tom Rini wrote:
> On Tue, Jul 23, 2013 at 01:34:15PM -0500, Dan Murphy wrote:
>> On 07/19/2013 02:00 PM, Tom Rini wrote:
>>> From: Greg Guyotte <gguyotte@ti.com>
>>>
>>> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
>>> family of boards.
Can we add the public reference to the technical manual in the commit message?
I found <http://www.ti.com/lit/ug/slvu580b/slvu580b.pdf>http://www.ti.com/lit/ds/symlink/tps65217b.pdf
>>>
>>> Signed-off-by: Greg Guyotte <gguyotte@ti.com>
>>> [trini: Split and rework Greg's changes into new drivers/power
>>> framework]
>>> Signed-off-by: Tom Rini <trini@ti.com>
>>> ---
>>> drivers/power/pmic/Makefile | 1 +
>>> drivers/power/pmic/pmic_tps65217.c | 108 ++++++++++++++++++++++++++++++++++++
>>> include/power/tps65217.h | 92 ++++++++++++++++++++++++++++++
>>> 3 files changed, 201 insertions(+)
>>> create mode 100644 drivers/power/pmic/pmic_tps65217.c
>>> create mode 100644 include/power/tps65217.h
>>>
>>> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
>>> index 14d426f..473cb80 100644
>>> --- a/drivers/power/pmic/Makefile
>>> +++ b/drivers/power/pmic/Makefile
>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
>>> COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
>>> COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
>>> COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
>>> +COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
>>>
>>> COBJS := $(COBJS-y)
>>> SRCS := $(COBJS:.o=.c)
>>> diff --git a/drivers/power/pmic/pmic_tps65217.c b/drivers/power/pmic/pmic_tps65217.c
>>> new file mode 100644
>>> index 0000000..c84bbcd
>>> --- /dev/null
>>> +++ b/drivers/power/pmic/pmic_tps65217.c
>>> @@ -0,0 +1,108 @@
>>> +/*
>>> + * (C) Copyright 2011-2013
>> Curious if this is the first time in why does it have a 2011 copyright?
> Because the code was written in 2011 (and has been whacked around a few
> times every year.
Got it thanks for the clarification
>
> [snip]
>>> +/**
>>> + * tps65217_reg_read() - Generic function that can read a TPS65217 register
>>> + * @src_reg: Source register address
>>> + * @src_val: Address of destination variable
>> No return defined here in the brief
> Fixed.
>
>>> + */
>>> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val)
>>> +{
>>> + if (i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1))
>>> + return 1;
>> This may be nit picky but generally in error cases we return negative.
>> Also why not return an error from errno?
> Because we're following i2c which is 0 or not 0, updated to use ret =
> i2c_read(...); if (ret) return ret here and throughout.
>
>> Also why an uchar when you are returning an int?
> Fixed.
>
> [snip]
>>> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
>> is prot_level a uchar or int?
> It's 0/1/2. I don't have a strong preference on if we type this out as
> an int or uchar.
>
>> Also would it not be better to have an interface that will check for
>> mask and do the read and just have a dedicated write function?
> I don't see the benefit, especially given the usage we have of just
> updating certain bitfields at a time.
>
> [snip]
>>> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)
>> No header for the interface
> Fixed.
>
>>> +{
>>> + if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
>>> + (dc_cntrl_reg != DEFDCDC3))
>> What do these magic numbers mean? Are these HEX numbers or a string?
> OK, it took me a minute to understand your question here. These are
> defines to register names, matching the TRM for the part. The register
> names are however annoyingly and easily confused as hex values.
Maybe we can rename the #defines so they are not so confusing. Maybe something like
#define TPS65217_<register name> <register offset>
So these will become
#define TPS65217_DEFDCDC1 0xe
#define TPS65217_DEFDCDC2 0xf
#define TPS65217_DEFDCDC3 0x10
This will at least keep other defines like CHIPID and SEQ unique as well.
>
>>> +#define PROT_LEVEL_NONE 0x00
>> Are these registers or a mask now?
>>> +#define PROT_LEVEL_1 0x01
>>> +#define PROT_LEVEL_2 0x02
> These are values as to what level of protection the chip has the
> register under.
>
>>> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val);
>>> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
>>> + uchar mask);
>>> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel);
>> Are these interfaces supposed to be accessed by an outside object?
>>
>> Typically there should be no direct register access from other objects.
> We can evaluate if there's consolidation to be done here once other
> boards go and adapt MPU clock frequency scaling. What registers need to
> be whacked on what board are going to decide if we can hide more
> details, or not.
>
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver
2013-07-25 17:21 ` Dan Murphy
@ 2013-07-25 18:05 ` Tom Rini
0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-25 18:05 UTC (permalink / raw)
To: u-boot
On Thu, Jul 25, 2013 at 12:21:41PM -0500, Dan Murphy wrote:
> On 07/25/2013 09:37 AM, Tom Rini wrote:
> > On Tue, Jul 23, 2013 at 01:34:15PM -0500, Dan Murphy wrote:
> >> On 07/19/2013 02:00 PM, Tom Rini wrote:
> >>> From: Greg Guyotte <gguyotte@ti.com>
> >>>
> >>> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> >>> family of boards.
>
> Can we add the public reference to the technical manual in the commit message?
>
> I found <http://www.ti.com/lit/ug/slvu580b/slvu580b.pdf>http://www.ti.com/lit/ds/symlink/tps65217b.pdf
I've got the stable link in the header file, now (also for tps65910).
[snip]
> >>> +{
> >>> + if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
> >>> + (dc_cntrl_reg != DEFDCDC3))
> >> What do these magic numbers mean? Are these HEX numbers or a string?
> > OK, it took me a minute to understand your question here. These are
> > defines to register names, matching the TRM for the part. The register
> > names are however annoyingly and easily confused as hex values.
>
> Maybe we can rename the #defines so they are not so confusing. Maybe
> something like
>
> #define TPS65217_<register name> <register offset>
>
> So these will become
> #define TPS65217_DEFDCDC1 0xe
> #define TPS65217_DEFDCDC2 0xf
> #define TPS65217_DEFDCDC3 0x10
>
> This will at least keep other defines like CHIPID and SEQ unique as well.
I can do that, yeah.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130725/52c1305e/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver
2013-07-19 19:00 [U-Boot] [PATCH 1/6] spl/Makefile: Add drivers/power/pmic/libpmic to CONFIG_SPL_POWER_SUPPORT Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver Tom Rini
@ 2013-07-19 19:00 ` Tom Rini
2013-07-25 17:41 ` Dan Murphy
2013-07-19 19:00 ` [U-Boot] [PATCH 4/6] am33xx: Add am33xx_spl_board_init function, call Tom Rini
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:00 UTC (permalink / raw)
To: u-boot
From: "Philip, Avinash" <avinashphilip@ti.com>
Add a driver for the TPS65910 PMIC that is found in the AM335x GP EVM,
AM335x EVM SK and others.
Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
[trini: Split and rework Avinash's changes into new drivers/power
framework]
Signed-off-by: Tom Rini <trini@ti.com>
---
drivers/power/pmic/Makefile | 1 +
drivers/power/pmic/pmic_tps65910.c | 69 +++++++++++++++++++++++++++++++
include/power/tps65910.h | 79 ++++++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+)
create mode 100644 drivers/power/pmic/pmic_tps65910.c
create mode 100644 include/power/tps65910.h
diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
index 473cb80..1811080 100644
--- a/drivers/power/pmic/Makefile
+++ b/drivers/power/pmic/Makefile
@@ -30,6 +30,7 @@ COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
+COBJS-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o
COBJS := $(COBJS-y)
SRCS := $(COBJS:.o=.c)
diff --git a/drivers/power/pmic/pmic_tps65910.c b/drivers/power/pmic/pmic_tps65910.c
new file mode 100644
index 0000000..0303f71
--- /dev/null
+++ b/drivers/power/pmic/pmic_tps65910.c
@@ -0,0 +1,69 @@
+/*
+ * (C) Copyright 2011-2013
+ * Texas Instruments, <www.ti.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 <i2c.h>
+#include <power/tps65910.h>
+
+/*
+ * voltage switching for MPU frequency switching.
+ * @module = mpu - 0, core - 1
+ * @vddx_op_vol_sel = vdd voltage to set
+ */
+int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel)
+{
+ uchar buf[4];
+ unsigned int reg_offset;
+
+ if (module == MPU)
+ reg_offset = TPS65910_VDD1_OP_REG;
+ else
+ reg_offset = TPS65910_VDD2_OP_REG;
+
+ /* Select VDDx OP */
+ if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
+ return 1;
+
+ buf[0] &= ~TPS65910_OP_REG_CMD_MASK;
+
+ if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
+ return 1;
+
+ /* Configure VDDx OP Voltage */
+ if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
+ return 1;
+
+ buf[0] &= ~TPS65910_OP_REG_SEL_MASK;
+ buf[0] |= vddx_op_vol_sel;
+
+ if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
+ return 1;
+
+ if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
+ return 1;
+
+ if ((buf[0] & TPS65910_OP_REG_SEL_MASK) != vddx_op_vol_sel)
+ return 1;
+
+ return 0;
+}
diff --git a/include/power/tps65910.h b/include/power/tps65910.h
new file mode 100644
index 0000000..5942721
--- /dev/null
+++ b/include/power/tps65910.h
@@ -0,0 +1,79 @@
+/*
+ * (C) Copyright 2011-2013
+ * Texas Instruments, <www.ti.com>
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef __POWER_TPS65910_H__
+#define __POWER_TPS65910_H__
+
+#define MPU 0
+#define CORE 1
+
+int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel);
+
+#define TPS65910_SR_I2C_ADDR 0x12
+#define TPS65910_CTRL_I2C_ADDR 0x2D
+
+/* PMIC Register offsets */
+#define TPS65910_VDD1_REG 0x21
+#define TPS65910_VDD1_OP_REG 0x22
+#define TPS65910_VDD2_REG 0x24
+#define TPS65910_VDD2_OP_REG 0x25
+#define TPS65910_DEVCTRL_REG 0x3F
+
+/* VDD2 & VDD1 control register (VDD2_REG & VDD1_REG) */
+#define TPS65910_VGAIN_SEL_MASK (0x3 << 6)
+#define TPS65910_ILMAX_MASK (0x1 << 5)
+#define TPS65910_TSTEP_MASK (0x7 << 2)
+#define TPS65910_ST_MASK (0x3)
+
+#define TPS65910_REG_VGAIN_SEL_X1 (0x0 << 6)
+#define TPS65910_REG_VGAIN_SEL_X1_0 (0x1 << 6)
+#define TPS65910_REG_VGAIN_SEL_X3 (0x2 << 6)
+#define TPS65910_REG_VGAIN_SEL_X4 (0x3 << 6)
+
+#define TPS65910_REG_ILMAX_1_0_A (0x0 << 5)
+#define TPS65910_REG_ILMAX_1_5_A (0x1 << 5)
+
+#define TPS65910_REG_TSTEP_ (0x0 << 2)
+#define TPS65910_REG_TSTEP_12_5 (0x1 << 2)
+#define TPS65910_REG_TSTEP_9_4 (0x2 << 2)
+#define TPS65910_REG_TSTEP_7_5 (0x3 << 2)
+#define TPS65910_REG_TSTEP_6_25 (0x4 << 2)
+#define TPS65910_REG_TSTEP_4_7 (0x5 << 2)
+#define TPS65910_REG_TSTEP_3_12 (0x6 << 2)
+#define TPS65910_REG_TSTEP_2_5 (0x7 << 2)
+
+#define TPS65910_REG_ST_OFF (0x0)
+#define TPS65910_REG_ST_ON_HI_POW (0x1)
+#define TPS65910_REG_ST_OFF_1 (0x2)
+#define TPS65910_REG_ST_ON_LOW_POW (0x3)
+
+
+/* VDD2 & VDD1 voltage selection register. (VDD2_OP_REG & VDD1_OP_REG) */
+#define TPS65910_OP_REG_SEL (0x7F)
+
+#define TPS65910_OP_REG_CMD_MASK (0x1 << 7)
+#define TPS65910_OP_REG_CMD_OP (0x0 << 7)
+#define TPS65910_OP_REG_CMD_SR (0x1 << 7)
+
+#define TPS65910_OP_REG_SEL_MASK (0x7F)
+#define TPS65910_OP_REG_SEL_0_9_5 (0x1F) /* 0.9500 V */
+#define TPS65910_OP_REG_SEL_1_1_3 (0x2E) /* 1.1375 V */
+#define TPS65910_OP_REG_SEL_1_2_0 (0x33) /* 1.2000 V */
+#define TPS65910_OP_REG_SEL_1_2_6 (0x38) /* 1.2625 V */
+#define TPS65910_OP_REG_SEL_1_3_2_5 (0x3D) /* 1.3250 V */
+
+/* Device control register . (DEVCTRL_REG) */
+#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_MASK (0x1 << 4)
+#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_SR_I2C (0x0 << 4)
+#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C (0x1 << 4)
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver
2013-07-19 19:00 ` [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver Tom Rini
@ 2013-07-25 17:41 ` Dan Murphy
2013-07-26 12:52 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2013-07-25 17:41 UTC (permalink / raw)
To: u-boot
On 07/19/2013 02:00 PM, Tom Rini wrote:
> From: "Philip, Avinash" <avinashphilip@ti.com>
>
> Add a driver for the TPS65910 PMIC that is found in the AM335x GP EVM,
> AM335x EVM SK and others.
Can we add a link to the technical manual?
http://www.ti.com/product/tps65910
> Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> [trini: Split and rework Avinash's changes into new drivers/power
> framework]
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/pmic_tps65910.c | 69 +++++++++++++++++++++++++++++++
> include/power/tps65910.h | 79 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/power/pmic/pmic_tps65910.c
> create mode 100644 include/power/tps65910.h
>
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 473cb80..1811080 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -30,6 +30,7 @@ COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
> COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
> COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
> COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
> +COBJS-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o
>
> COBJS := $(COBJS-y)
> SRCS := $(COBJS:.o=.c)
> diff --git a/drivers/power/pmic/pmic_tps65910.c b/drivers/power/pmic/pmic_tps65910.c
> new file mode 100644
> index 0000000..0303f71
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65910.c
> @@ -0,0 +1,69 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.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 <i2c.h>
> +#include <power/tps65910.h>
> +
> +/*
> + * voltage switching for MPU frequency switching.
> + * @module = mpu - 0, core - 1
> + * @vddx_op_vol_sel = vdd voltage to set
No return identified here
> + */
> +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel)
> +{
> + uchar buf[4];
If we only read and write one byte at a time why is this an array of 4?
> + unsigned int reg_offset;
> +
> + if (module == MPU)
> + reg_offset = TPS65910_VDD1_OP_REG;
> + else
> + reg_offset = TPS65910_VDD2_OP_REG;
This seems very specific to the implementation.
Can VDD2 ever be used for the MPU? Or are there constraints on the VDD2 SMPS that will not allow that?
> +
> + /* Select VDDx OP */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
I am going to assume that you changed this as well like you did for the TPS65217?
> +
> + buf[0] &= ~TPS65910_OP_REG_CMD_MASK;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + /* Configure VDDx OP Voltage */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + buf[0] &= ~TPS65910_OP_REG_SEL_MASK;
> + buf[0] |= vddx_op_vol_sel;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + if ((buf[0] & TPS65910_OP_REG_SEL_MASK) != vddx_op_vol_sel)
> + return 1;
> +
> + return 0;
> +}
> diff --git a/include/power/tps65910.h b/include/power/tps65910.h
> new file mode 100644
> index 0000000..5942721
> --- /dev/null
> +++ b/include/power/tps65910.h
> @@ -0,0 +1,79 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.com>
> + *
> + * 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 version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __POWER_TPS65910_H__
> +#define __POWER_TPS65910_H__
> +
> +#define MPU 0
> +#define CORE 1
> +
> +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel);
Nitpick - Don't the interface definitions go at the end of the file?
> +
> +#define TPS65910_SR_I2C_ADDR 0x12
> +#define TPS65910_CTRL_I2C_ADDR 0x2D
> +
> +/* PMIC Register offsets */
> +#define TPS65910_VDD1_REG 0x21
> +#define TPS65910_VDD1_OP_REG 0x22
> +#define TPS65910_VDD2_REG 0x24
> +#define TPS65910_VDD2_OP_REG 0x25
> +#define TPS65910_DEVCTRL_REG 0x3F
> +
> +/* VDD2 & VDD1 control register (VDD2_REG & VDD1_REG) */
> +#define TPS65910_VGAIN_SEL_MASK (0x3 << 6)
> +#define TPS65910_ILMAX_MASK (0x1 << 5)
> +#define TPS65910_TSTEP_MASK (0x7 << 2)
> +#define TPS65910_ST_MASK (0x3)
> +
> +#define TPS65910_REG_VGAIN_SEL_X1 (0x0 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X1_0 (0x1 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X3 (0x2 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X4 (0x3 << 6)
> +
> +#define TPS65910_REG_ILMAX_1_0_A (0x0 << 5)
> +#define TPS65910_REG_ILMAX_1_5_A (0x1 << 5)
> +
> +#define TPS65910_REG_TSTEP_ (0x0 << 2)
> +#define TPS65910_REG_TSTEP_12_5 (0x1 << 2)
> +#define TPS65910_REG_TSTEP_9_4 (0x2 << 2)
> +#define TPS65910_REG_TSTEP_7_5 (0x3 << 2)
> +#define TPS65910_REG_TSTEP_6_25 (0x4 << 2)
> +#define TPS65910_REG_TSTEP_4_7 (0x5 << 2)
> +#define TPS65910_REG_TSTEP_3_12 (0x6 << 2)
> +#define TPS65910_REG_TSTEP_2_5 (0x7 << 2)
> +
> +#define TPS65910_REG_ST_OFF (0x0)
> +#define TPS65910_REG_ST_ON_HI_POW (0x1)
> +#define TPS65910_REG_ST_OFF_1 (0x2)
> +#define TPS65910_REG_ST_ON_LOW_POW (0x3)
> +
> +
Extra line
> +/* VDD2 & VDD1 voltage selection register. (VDD2_OP_REG & VDD1_OP_REG) */
> +#define TPS65910_OP_REG_SEL (0x7F)
> +
> +#define TPS65910_OP_REG_CMD_MASK (0x1 << 7)
> +#define TPS65910_OP_REG_CMD_OP (0x0 << 7)
> +#define TPS65910_OP_REG_CMD_SR (0x1 << 7)
> +
> +#define TPS65910_OP_REG_SEL_MASK (0x7F)
> +#define TPS65910_OP_REG_SEL_0_9_5 (0x1F) /* 0.9500 V */
> +#define TPS65910_OP_REG_SEL_1_1_3 (0x2E) /* 1.1375 V */
> +#define TPS65910_OP_REG_SEL_1_2_0 (0x33) /* 1.2000 V */
> +#define TPS65910_OP_REG_SEL_1_2_6 (0x38) /* 1.2625 V */
> +#define TPS65910_OP_REG_SEL_1_3_2_5 (0x3D) /* 1.3250 V */
> +
> +/* Device control register . (DEVCTRL_REG) */
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_MASK (0x1 << 4)
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_SR_I2C (0x0 << 4)
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C (0x1 << 4)
> +#endif
Nitpick - #endif /* __POWER_TPS65910_H_ */
Is this not a family of parts?
http://www.ti.com/product/tps65910
TPS65910, TPS65910A, TPS65910A3, TPS659101, TPS659102, TPS659103
TPS659104, TPS659105, TPS659106, TPS659107, TPS659108, TPS659109
Do you think we could rename the definitions and the file to TPS65910x?
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver
2013-07-25 17:41 ` Dan Murphy
@ 2013-07-26 12:52 ` Tom Rini
0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-26 12:52 UTC (permalink / raw)
To: u-boot
On Thu, Jul 25, 2013 at 12:41:46PM -0500, Dan Murphy wrote:
> On 07/19/2013 02:00 PM, Tom Rini wrote:
> > From: "Philip, Avinash" <avinashphilip@ti.com>
> >
> > Add a driver for the TPS65910 PMIC that is found in the AM335x GP EVM,
> > AM335x EVM SK and others.
>
> Can we add a link to the technical manual?
> http://www.ti.com/product/tps65910
Yup, in the code.
[snip]
> > +/*
> > + * voltage switching for MPU frequency switching.
> > + * @module = mpu - 0, core - 1
> > + * @vddx_op_vol_sel = vdd voltage to set
>
> No return identified here
Fixed.
> > + */
> > +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel)
> > +{
> > + uchar buf[4];
>
> If we only read and write one byte at a time why is this an array of 4?
Fixed.
> > + unsigned int reg_offset;
> > +
> > + if (module == MPU)
> > + reg_offset = TPS65910_VDD1_OP_REG;
> > + else
> > + reg_offset = TPS65910_VDD2_OP_REG;
>
> This seems very specific to the implementation.
>
> Can VDD2 ever be used for the MPU? Or are there constraints on the
> VDD2 SMPS that will not allow that?
Can? Probably. Will someone deviate from the reference design?
Probably not.
> > +
> > + /* Select VDDx OP */
> > + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> > + return 1;
>
> I am going to assume that you changed this as well like you did for
> the TPS65217?
Correct.
> > +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel);
>
> Nitpick - Don't the interface definitions go at the end of the file?
Sure.
[snip]
> Is this not a family of parts?
>
> http://www.ti.com/product/tps65910
>
> TPS65910, TPS65910A, TPS65910A3, TPS659101, TPS659102, TPS659103
> TPS659104, TPS659105, TPS659106, TPS659107, TPS659108, TPS659109
>
> Do you think we could rename the definitions and the file to TPS65910x?
Fixed the other two minor things, and yes, but if we follow the kernel
example here (note that we didn't copy code) it's still just tps65910
and tps65217 (tps65217 has a/b/c variants).
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130726/82ca76c7/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH 4/6] am33xx: Add am33xx_spl_board_init function, call
2013-07-19 19:00 [U-Boot] [PATCH 1/6] spl/Makefile: Add drivers/power/pmic/libpmic to CONFIG_SPL_POWER_SUPPORT Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver Tom Rini
@ 2013-07-19 19:00 ` Tom Rini
2013-07-22 14:41 ` [U-Boot] [PATCH v2 " Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 5/6] am33xx: Add the efuse_sma CONTROL_MODULE register Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency Tom Rini
4 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:00 UTC (permalink / raw)
To: u-boot
We need to allow for a further call-out in spl_board_init. Call this
am33xx_spl_board_init and add a __weak version. This function may be
used to scale the MPU frequency up, depending on board needs.
Signed-off-by: Tom Rini <trini@ti.com>
---
arch/arm/cpu/armv7/omap-common/boot-common.c | 12 ++++++++++++
arch/arm/include/asm/arch-am33xx/sys_proto.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/omap-common/boot-common.c b/arch/arm/cpu/armv7/omap-common/boot-common.c
index 76ae1b6..bf11f04 100644
--- a/arch/arm/cpu/armv7/omap-common/boot-common.c
+++ b/arch/arm/cpu/armv7/omap-common/boot-common.c
@@ -22,6 +22,7 @@
#include <asm/arch/omap.h>
#include <asm/arch/mmc_host_def.h>
#include <asm/arch/sys_proto.h>
+#include <linux/compiler.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -75,6 +76,14 @@ u32 spl_boot_mode(void)
return gd->arch.omap_boot_params.omap_bootmode;
}
+/*
+ * This function is the place to do per-board things such as ramp up the
+ * MPU clock frequency.
+ */
+__weak void am33xx_spl_board_init(void)
+{
+}
+
void spl_board_init(void)
{
#ifdef CONFIG_SPL_NAND_SUPPORT
@@ -83,6 +92,9 @@ void spl_board_init(void)
#if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW_SUPPORT)
arch_misc_init();
#endif
+#ifdef CONFIG_AM33XX
+ am33xx_spl_board_init();
+#endif
}
int board_mmc_init(bd_t *bis)
diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h
index 307ac28..98dc830 100644
--- a/arch/arm/include/asm/arch-am33xx/sys_proto.h
+++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h
@@ -45,4 +45,5 @@ void omap_nand_switch_ecc(uint32_t, uint32_t);
void rtc32k_enable(void);
void uart_soft_reset(void);
+void am33xx_spl_board_init(void);
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH v2 4/6] am33xx: Add am33xx_spl_board_init function, call
2013-07-19 19:00 ` [U-Boot] [PATCH 4/6] am33xx: Add am33xx_spl_board_init function, call Tom Rini
@ 2013-07-22 14:41 ` Tom Rini
0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-22 14:41 UTC (permalink / raw)
To: u-boot
We need to allow for a further call-out in spl_board_init. Call this
am33xx_spl_board_init and add a __weak version. This function may be
used to scale the MPU frequency up, depending on board needs.
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v2:
- Move am33xx_spl_board_init to am33xx/board.c from
omap-common/boot-common.c
---
arch/arm/cpu/armv7/am33xx/board.c | 9 +++++++++
arch/arm/cpu/armv7/omap-common/boot-common.c | 3 +++
arch/arm/include/asm/arch-am33xx/sys_proto.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
index b935a29..9356501 100644
--- a/arch/arm/cpu/armv7/am33xx/board.c
+++ b/arch/arm/cpu/armv7/am33xx/board.c
@@ -35,6 +35,7 @@
#include <miiphy.h>
#include <cpsw.h>
#include <asm/errno.h>
+#include <linux/compiler.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
#include <linux/usb/musb.h>
@@ -151,6 +152,14 @@ int arch_misc_init(void)
}
#ifdef CONFIG_SPL_BUILD
+/*
+ * This function is the place to do per-board things such as ramp up the
+ * MPU clock frequency.
+ */
+__weak void am33xx_spl_board_init(void)
+{
+}
+
void rtc32k_enable(void)
{
struct rtc_regs *rtc = (struct rtc_regs *)RTC_BASE;
diff --git a/arch/arm/cpu/armv7/omap-common/boot-common.c b/arch/arm/cpu/armv7/omap-common/boot-common.c
index 76ae1b6..6671a29 100644
--- a/arch/arm/cpu/armv7/omap-common/boot-common.c
+++ b/arch/arm/cpu/armv7/omap-common/boot-common.c
@@ -83,6 +83,9 @@ void spl_board_init(void)
#if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW_SUPPORT)
arch_misc_init();
#endif
+#ifdef CONFIG_AM33XX
+ am33xx_spl_board_init();
+#endif
}
int board_mmc_init(bd_t *bis)
diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h
index 307ac28..98dc830 100644
--- a/arch/arm/include/asm/arch-am33xx/sys_proto.h
+++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h
@@ -45,4 +45,5 @@ void omap_nand_switch_ecc(uint32_t, uint32_t);
void rtc32k_enable(void);
void uart_soft_reset(void);
+void am33xx_spl_board_init(void);
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH 5/6] am33xx: Add the efuse_sma CONTROL_MODULE register
2013-07-19 19:00 [U-Boot] [PATCH 1/6] spl/Makefile: Add drivers/power/pmic/libpmic to CONFIG_SPL_POWER_SUPPORT Tom Rini
` (2 preceding siblings ...)
2013-07-19 19:00 ` [U-Boot] [PATCH 4/6] am33xx: Add am33xx_spl_board_init function, call Tom Rini
@ 2013-07-19 19:00 ` Tom Rini
2013-07-19 19:00 ` [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency Tom Rini
4 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:00 UTC (permalink / raw)
To: u-boot
Starting with PG2.1 we have a register in the CONTROL_MODULE that is set
with the package type and maximum supported frequency. Add this, and
the relevant mask/values.
Signed-off-by: Tom Rini <trini@ti.com>
---
arch/arm/include/asm/arch-am33xx/cpu.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
index 3d3a7c8..0288b8a 100644
--- a/arch/arm/include/asm/arch-am33xx/cpu.h
+++ b/arch/arm/include/asm/arch-am33xx/cpu.h
@@ -46,6 +46,16 @@
#define AM335X 0xB944
#define TI81XX 0xB81E
#define DEVICE_ID (CTRL_BASE + 0x0600)
+#define DEVICE_ID_MASK 0x1FFF
+
+/* MPU max frequencies */
+#define AM335X_ZCZ_300 0x1FEF
+#define AM335X_ZCZ_600 0x1FAF
+#define AM335X_ZCZ_720 0x1F2F
+#define AM335X_ZCZ_800 0x1E2F
+#define AM335X_ZCZ_1000 0x1C2F
+#define AM335X_ZCE_300 0x1FDF
+#define AM335X_ZCE_600 0x1F9F
/* This gives the status of the boot mode pins on the evm */
#define SYSBOOT_MASK (BIT(0) | BIT(1) | BIT(2)\
@@ -343,6 +353,8 @@ struct ctrl_dev {
unsigned int macid1h; /* offset 0x3c */
unsigned int resv4[4];
unsigned int miisel; /* offset 0x50 */
+ unsigned int resv5[106];
+ unsigned int efuse_sma; /* offset 0x1FC */
};
#endif /* __ASSEMBLY__ */
#endif /* __KERNEL_STRICT_NAMES */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-19 19:00 [U-Boot] [PATCH 1/6] spl/Makefile: Add drivers/power/pmic/libpmic to CONFIG_SPL_POWER_SUPPORT Tom Rini
` (3 preceding siblings ...)
2013-07-19 19:00 ` [U-Boot] [PATCH 5/6] am33xx: Add the efuse_sma CONTROL_MODULE register Tom Rini
@ 2013-07-19 19:00 ` Tom Rini
2013-07-19 19:37 ` Enric Balletbo Serra
2013-07-23 18:46 ` Dan Murphy
4 siblings, 2 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:00 UTC (permalink / raw)
To: u-boot
Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
depending on the board we are running on. In all cases, we see if we
can rely on the efuse_sma register to tell us the maximum speed. In the
case of Beaglebone White, we need to make sure we are on AC power, and
are on later than rev A1, and then we can ramp up to the PG1.0 maximum
of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
need to rely on the efuse_sma if we are on PG2.1, and the defaults for
PG1.0/2.0.
Signed-off-by: Tom Rini <trini@ti.com>
---
arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
board/ti/am335x/board.c | 155 ++++++++++++++++++++++
include/configs/am335x_evm.h | 4 +
3 files changed, 167 insertions(+)
diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
index 89b63d9..834f24f 100644
--- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
+++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
@@ -24,6 +24,14 @@
#define CONFIG_SYS_MPUCLK 550
#endif
+/* MAIN PLL Fdll supported frequencies */
+#define MPUPLL_M_1000 1000
+#define MPUPLL_M_800 800
+#define MPUPLL_M_720 720
+#define MPUPLL_M_600 600
+#define MPUPLL_M_550 550
+#define MPUPLL_M_300 300
+
extern void pll_init(void);
extern void enable_emif_clocks(void);
extern void enable_dmm_clocks(void);
diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
index fdbe26c..6544931 100644
--- a/board/ti/am335x/board.c
+++ b/board/ti/am335x/board.c
@@ -33,6 +33,8 @@
#include <i2c.h>
#include <miiphy.h>
#include <cpsw.h>
+#include <power/tps65217.h>
+#include <power/tps65910.h>
#include "board.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -282,6 +284,159 @@ int spl_start_uboot(void)
}
#endif
+void am33xx_spl_board_init(void)
+{
+ int mpu_vdd, mpu_pll, sil_rev;
+
+ /* Assume PG 1.0 */
+ mpu_pll = MPUPLL_M_720;
+
+ sil_rev = readl(&cdev->deviceid) >> 28;
+ if (sil_rev == 1)
+ /* PG 2.0, efuse may not be set. */
+ mpu_pll = MPUPLL_M_800;
+ else if (sil_rev >= 2) {
+ /* Check what the efuse says our max speed is. */
+ int efuse_arm_mpu_max_freq;
+ efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
+ switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
+ case AM335X_ZCZ_1000:
+ mpu_pll = MPUPLL_M_1000;
+ break;
+ case AM335X_ZCZ_800:
+ mpu_pll = MPUPLL_M_800;
+ break;
+ case AM335X_ZCZ_720:
+ mpu_pll = MPUPLL_M_720;
+ break;
+ case AM335X_ZCZ_600:
+ case AM335X_ZCE_600:
+ mpu_pll = MPUPLL_M_600;
+ break;
+ case AM335X_ZCZ_300:
+ case AM335X_ZCE_300:
+ mpu_pll = MPUPLL_M_300;
+ break;
+ }
+ }
+
+ if (board_is_bone() || board_is_bone_lt()) {
+ /* BeagleBone PMIC Code */
+ int usb_cur_lim;
+
+ /*
+ * Only perform PMIC configurations if board rev > A1
+ * on Beaglebone White
+ */
+ if (board_is_bone() && !strncmp(header.version, "00A1", 4))
+ return;
+
+ if (i2c_probe(TPS65217_CHIP_PM))
+ return;
+
+ /*
+ * On Beaglebone White we need to ensure we have AC power
+ * before increasing the frequency.
+ */
+ if (board_is_bone()) {
+ uchar pmic_status_reg;
+ if (tps65217_reg_read(STATUS, &pmic_status_reg))
+ return;
+ if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
+ puts("No AC power, disabling frequency switch\n");
+ return;
+ }
+ }
+
+ /*
+ * Increase USB current limit to 1300mA or 1800mA and set
+ * the MPU voltage controller as needed.
+ */
+ if (mpu_pll == MPUPLL_M_1000) {
+ usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
+ mpu_vdd = DCDC_VOLT_SEL_1325MV;
+ } else {
+ usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
+ mpu_vdd = DCDC_VOLT_SEL_1275MV;
+ }
+
+ if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
+ usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
+ printf("tps65217_reg_write failure\n");
+
+
+ /* Set DCDC2 (MPU) voltage */
+ if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
+ printf("tps65217_voltage_update failure\n");
+ return;
+ }
+
+ /* Set LDO3, LDO4 output voltage to 3.3V */
+ if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
+ LDO_VOLTAGE_OUT_3_3, LDO_MASK))
+ printf("tps65217_reg_write failure\n");
+
+ if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
+ LDO_VOLTAGE_OUT_3_3, LDO_MASK))
+ printf("tps65217_reg_write failure\n");
+ } else {
+ uchar buf[4];
+
+ /*
+ * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
+ * MPU frequencies we support we use a CORE voltage of
+ * 1.1375V. For MPU voltage we need to switch based on
+ * the frequency we are running at.
+ */
+ if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
+ return;
+
+ /* VDD1/2 voltage selection register access by control i/f */
+ if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
+ buf, 1))
+ return;
+
+ buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
+
+ if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
+ buf, 1))
+ return;
+
+ /* Depending on MPU clock we need different MPU VDD */
+
+ /* Default to PG1.0/PG2.0 values. */
+ mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
+
+ if (sil_rev >= 2) {
+ switch (mpu_pll) {
+ case MPUPLL_M_1000:
+ mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
+ break;
+ case MPUPLL_M_800:
+ mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
+ break;
+ case MPUPLL_M_720:
+ mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
+ break;
+ case MPUPLL_M_600:
+ case MPUPLL_M_300:
+ mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
+ break;
+ }
+ }
+
+ /* First update MPU voltage. */
+ if (tps65910_voltage_update(MPU, mpu_vdd))
+ return;
+
+ /* Second, update the CORE voltage. */
+ if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
+ return;
+ }
+
+ /* Set MPU Frequency to what we detected now that voltages are set */
+ mpu_pll_config_val(mpu_pll);
+}
#endif
/*
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index c5a6d4b..60c87a3 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -289,6 +289,9 @@
#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
#define CONFIG_SYS_I2C_MULTI_EEPROMS
+#define CONFIG_POWER_TPS65217
+#define CONFIG_POWER_TPS65910
+
#define CONFIG_OMAP_GPIO
#define CONFIG_BAUDRATE 115200
@@ -352,6 +355,7 @@
#define CONFIG_SPL_LIBCOMMON_SUPPORT
#define CONFIG_SPL_LIBDISK_SUPPORT
#define CONFIG_SPL_LIBGENERIC_SUPPORT
+#define CONFIG_SPL_POWER_SUPPORT
#define CONFIG_SPL_SERIAL_SUPPORT
#define CONFIG_SPL_GPIO_SUPPORT
#define CONFIG_SPL_YMODEM_SUPPORT
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-19 19:00 ` [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency Tom Rini
@ 2013-07-19 19:37 ` Enric Balletbo Serra
2013-07-19 19:48 ` Tom Rini
2013-07-23 18:46 ` Dan Murphy
1 sibling, 1 reply; 21+ messages in thread
From: Enric Balletbo Serra @ 2013-07-19 19:37 UTC (permalink / raw)
To: u-boot
Hi Tom,
Just a comment below ...
2013/7/19 Tom Rini <trini@ti.com>:
> Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
> depending on the board we are running on. In all cases, we see if we
> can rely on the efuse_sma register to tell us the maximum speed. In the
> case of Beaglebone White, we need to make sure we are on AC power, and
> are on later than rev A1, and then we can ramp up to the PG1.0 maximum
> of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
> supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
> cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
> need to rely on the efuse_sma if we are on PG2.1, and the defaults for
> PG1.0/2.0.
>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
> board/ti/am335x/board.c | 155 ++++++++++++++++++++++
> include/configs/am335x_evm.h | 4 +
> 3 files changed, 167 insertions(+)
>
> diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> index 89b63d9..834f24f 100644
> --- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> +++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> @@ -24,6 +24,14 @@
> #define CONFIG_SYS_MPUCLK 550
> #endif
>
> +/* MAIN PLL Fdll supported frequencies */
> +#define MPUPLL_M_1000 1000
> +#define MPUPLL_M_800 800
> +#define MPUPLL_M_720 720
> +#define MPUPLL_M_600 600
> +#define MPUPLL_M_550 550
> +#define MPUPLL_M_300 300
> +
> extern void pll_init(void);
> extern void enable_emif_clocks(void);
> extern void enable_dmm_clocks(void);
> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> index fdbe26c..6544931 100644
> --- a/board/ti/am335x/board.c
> +++ b/board/ti/am335x/board.c
> @@ -33,6 +33,8 @@
> #include <i2c.h>
> #include <miiphy.h>
> #include <cpsw.h>
> +#include <power/tps65217.h>
> +#include <power/tps65910.h>
> #include "board.h"
>
> DECLARE_GLOBAL_DATA_PTR;
> @@ -282,6 +284,159 @@ int spl_start_uboot(void)
> }
> #endif
>
> +void am33xx_spl_board_init(void)
> +{
> + int mpu_vdd, mpu_pll, sil_rev;
> +
> + /* Assume PG 1.0 */
> + mpu_pll = MPUPLL_M_720;
> +
> + sil_rev = readl(&cdev->deviceid) >> 28;
> + if (sil_rev == 1)
> + /* PG 2.0, efuse may not be set. */
> + mpu_pll = MPUPLL_M_800;
> + else if (sil_rev >= 2) {
> + /* Check what the efuse says our max speed is. */
> + int efuse_arm_mpu_max_freq;
> + efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
> + switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
> + case AM335X_ZCZ_1000:
> + mpu_pll = MPUPLL_M_1000;
> + break;
> + case AM335X_ZCZ_800:
> + mpu_pll = MPUPLL_M_800;
> + break;
> + case AM335X_ZCZ_720:
> + mpu_pll = MPUPLL_M_720;
> + break;
> + case AM335X_ZCZ_600:
> + case AM335X_ZCE_600:
> + mpu_pll = MPUPLL_M_600;
> + break;
> + case AM335X_ZCZ_300:
> + case AM335X_ZCE_300:
> + mpu_pll = MPUPLL_M_300;
> + break;
> + }
> + }
> +
> + if (board_is_bone() || board_is_bone_lt()) {
> + /* BeagleBone PMIC Code */
> + int usb_cur_lim;
> +
> + /*
> + * Only perform PMIC configurations if board rev > A1
> + * on Beaglebone White
> + */
> + if (board_is_bone() && !strncmp(header.version, "00A1", 4))
> + return;
> +
> + if (i2c_probe(TPS65217_CHIP_PM))
> + return;
> +
> + /*
> + * On Beaglebone White we need to ensure we have AC power
> + * before increasing the frequency.
> + */
> + if (board_is_bone()) {
> + uchar pmic_status_reg;
> + if (tps65217_reg_read(STATUS, &pmic_status_reg))
> + return;
> + if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
> + puts("No AC power, disabling frequency switch\n");
> + return;
> + }
> + }
> +
> + /*
> + * Increase USB current limit to 1300mA or 1800mA and set
> + * the MPU voltage controller as needed.
> + */
> + if (mpu_pll == MPUPLL_M_1000) {
> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
> + mpu_vdd = DCDC_VOLT_SEL_1325MV;
> + } else {
> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
> + mpu_vdd = DCDC_VOLT_SEL_1275MV;
> + }
> +
> + if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
> + usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
> + printf("tps65217_reg_write failure\n");
> +
> +
> + /* Set DCDC2 (MPU) voltage */
> + if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
> + printf("tps65217_voltage_update failure\n");
> + return;
> + }
> +
> + /* Set LDO3, LDO4 output voltage to 3.3V */
> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> + printf("tps65217_reg_write failure\n");
> +
> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> + printf("tps65217_reg_write failure\n");
> + } else {
> + uchar buf[4];
> +
> + /*
> + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
> + * MPU frequencies we support we use a CORE voltage of
> + * 1.1375V. For MPU voltage we need to switch based on
> + * the frequency we are running at.
> + */
> + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
> + return;
> +
> + /* VDD1/2 voltage selection register access by control i/f */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> + buf, 1))
> + return;
> +
> + buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> + buf, 1))
> + return;
> +
> + /* Depending on MPU clock we need different MPU VDD */
> +
> + /* Default to PG1.0/PG2.0 values. */
> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> +
> + if (sil_rev >= 2) {
> + switch (mpu_pll) {
> + case MPUPLL_M_1000:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
> + break;
> + case MPUPLL_M_800:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
> + break;
> + case MPUPLL_M_720:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
> + break;
> + case MPUPLL_M_600:
> + case MPUPLL_M_300:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> + break;
> + }
> + }
> +
> + /* First update MPU voltage. */
> + if (tps65910_voltage_update(MPU, mpu_vdd))
> + return;
> +
> + /* Second, update the CORE voltage. */
> + if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
> + return;
> + }
> +
> + /* Set MPU Frequency to what we detected now that voltages are set */
> + mpu_pll_config_val(mpu_pll);
> +}
> #endif
>
Would be possible rework this function in order to be more generic ?
Or maybe split the parts that are common from the parts that are board
specific ?
I think part of this code can be shared for other boards that use
AM335x processor ( like IGEP COM AQUILA.)
> /*
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index c5a6d4b..60c87a3 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -289,6 +289,9 @@
> #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
> #define CONFIG_SYS_I2C_MULTI_EEPROMS
>
> +#define CONFIG_POWER_TPS65217
> +#define CONFIG_POWER_TPS65910
> +
> #define CONFIG_OMAP_GPIO
>
> #define CONFIG_BAUDRATE 115200
> @@ -352,6 +355,7 @@
> #define CONFIG_SPL_LIBCOMMON_SUPPORT
> #define CONFIG_SPL_LIBDISK_SUPPORT
> #define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_POWER_SUPPORT
> #define CONFIG_SPL_SERIAL_SUPPORT
> #define CONFIG_SPL_GPIO_SUPPORT
> #define CONFIG_SPL_YMODEM_SUPPORT
> --
> 1.7.9.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
CC'ing Javier Martinez Canillas <martinez.javier@gmail.com>
Best regards,
Enric
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-19 19:37 ` Enric Balletbo Serra
@ 2013-07-19 19:48 ` Tom Rini
2013-07-19 20:58 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2013-07-19 19:48 UTC (permalink / raw)
To: u-boot
On Fri, Jul 19, 2013 at 09:37:51PM +0200, Enric Balletbo Serra wrote:
> Hi Tom,
>
> Just a comment below ...
>
> 2013/7/19 Tom Rini <trini@ti.com>:
> > Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
> > depending on the board we are running on. In all cases, we see if we
> > can rely on the efuse_sma register to tell us the maximum speed. In the
> > case of Beaglebone White, we need to make sure we are on AC power, and
> > are on later than rev A1, and then we can ramp up to the PG1.0 maximum
> > of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
> > supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
> > cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
> > need to rely on the efuse_sma if we are on PG2.1, and the defaults for
> > PG1.0/2.0.
> >
> > Signed-off-by: Tom Rini <trini@ti.com>
> > ---
> > arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
> > board/ti/am335x/board.c | 155 ++++++++++++++++++++++
> > include/configs/am335x_evm.h | 4 +
> > 3 files changed, 167 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > index 89b63d9..834f24f 100644
> > --- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > +++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > @@ -24,6 +24,14 @@
> > #define CONFIG_SYS_MPUCLK 550
> > #endif
> >
> > +/* MAIN PLL Fdll supported frequencies */
> > +#define MPUPLL_M_1000 1000
> > +#define MPUPLL_M_800 800
> > +#define MPUPLL_M_720 720
> > +#define MPUPLL_M_600 600
> > +#define MPUPLL_M_550 550
> > +#define MPUPLL_M_300 300
> > +
> > extern void pll_init(void);
> > extern void enable_emif_clocks(void);
> > extern void enable_dmm_clocks(void);
> > diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> > index fdbe26c..6544931 100644
> > --- a/board/ti/am335x/board.c
> > +++ b/board/ti/am335x/board.c
> > @@ -33,6 +33,8 @@
> > #include <i2c.h>
> > #include <miiphy.h>
> > #include <cpsw.h>
> > +#include <power/tps65217.h>
> > +#include <power/tps65910.h>
> > #include "board.h"
> >
> > DECLARE_GLOBAL_DATA_PTR;
> > @@ -282,6 +284,159 @@ int spl_start_uboot(void)
> > }
> > #endif
> >
> > +void am33xx_spl_board_init(void)
> > +{
> > + int mpu_vdd, mpu_pll, sil_rev;
> > +
> > + /* Assume PG 1.0 */
> > + mpu_pll = MPUPLL_M_720;
> > +
> > + sil_rev = readl(&cdev->deviceid) >> 28;
> > + if (sil_rev == 1)
> > + /* PG 2.0, efuse may not be set. */
> > + mpu_pll = MPUPLL_M_800;
> > + else if (sil_rev >= 2) {
> > + /* Check what the efuse says our max speed is. */
> > + int efuse_arm_mpu_max_freq;
> > + efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
> > + switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
> > + case AM335X_ZCZ_1000:
> > + mpu_pll = MPUPLL_M_1000;
> > + break;
> > + case AM335X_ZCZ_800:
> > + mpu_pll = MPUPLL_M_800;
> > + break;
> > + case AM335X_ZCZ_720:
> > + mpu_pll = MPUPLL_M_720;
> > + break;
> > + case AM335X_ZCZ_600:
> > + case AM335X_ZCE_600:
> > + mpu_pll = MPUPLL_M_600;
> > + break;
> > + case AM335X_ZCZ_300:
> > + case AM335X_ZCE_300:
> > + mpu_pll = MPUPLL_M_300;
> > + break;
> > + }
> > + }
> > +
> > + if (board_is_bone() || board_is_bone_lt()) {
> > + /* BeagleBone PMIC Code */
> > + int usb_cur_lim;
> > +
> > + /*
> > + * Only perform PMIC configurations if board rev > A1
> > + * on Beaglebone White
> > + */
> > + if (board_is_bone() && !strncmp(header.version, "00A1", 4))
> > + return;
> > +
> > + if (i2c_probe(TPS65217_CHIP_PM))
> > + return;
> > +
> > + /*
> > + * On Beaglebone White we need to ensure we have AC power
> > + * before increasing the frequency.
> > + */
> > + if (board_is_bone()) {
> > + uchar pmic_status_reg;
> > + if (tps65217_reg_read(STATUS, &pmic_status_reg))
> > + return;
> > + if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
> > + puts("No AC power, disabling frequency switch\n");
> > + return;
> > + }
> > + }
> > +
> > + /*
> > + * Increase USB current limit to 1300mA or 1800mA and set
> > + * the MPU voltage controller as needed.
> > + */
> > + if (mpu_pll == MPUPLL_M_1000) {
> > + usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
> > + mpu_vdd = DCDC_VOLT_SEL_1325MV;
> > + } else {
> > + usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
> > + mpu_vdd = DCDC_VOLT_SEL_1275MV;
> > + }
> > +
> > + if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
> > + usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
> > + printf("tps65217_reg_write failure\n");
> > +
> > +
> > + /* Set DCDC2 (MPU) voltage */
> > + if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
> > + printf("tps65217_voltage_update failure\n");
> > + return;
> > + }
> > +
> > + /* Set LDO3, LDO4 output voltage to 3.3V */
> > + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
> > + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> > + printf("tps65217_reg_write failure\n");
> > +
> > + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
> > + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> > + printf("tps65217_reg_write failure\n");
> > + } else {
> > + uchar buf[4];
> > +
> > + /*
> > + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
> > + * MPU frequencies we support we use a CORE voltage of
> > + * 1.1375V. For MPU voltage we need to switch based on
> > + * the frequency we are running at.
> > + */
> > + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
> > + return;
> > +
> > + /* VDD1/2 voltage selection register access by control i/f */
> > + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> > + buf, 1))
> > + return;
> > +
> > + buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
> > +
> > + if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> > + buf, 1))
> > + return;
> > +
> > + /* Depending on MPU clock we need different MPU VDD */
> > +
> > + /* Default to PG1.0/PG2.0 values. */
> > + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> > +
> > + if (sil_rev >= 2) {
> > + switch (mpu_pll) {
> > + case MPUPLL_M_1000:
> > + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
> > + break;
> > + case MPUPLL_M_800:
> > + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
> > + break;
> > + case MPUPLL_M_720:
> > + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
> > + break;
> > + case MPUPLL_M_600:
> > + case MPUPLL_M_300:
> > + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> > + break;
> > + }
> > + }
> > +
> > + /* First update MPU voltage. */
> > + if (tps65910_voltage_update(MPU, mpu_vdd))
> > + return;
> > +
> > + /* Second, update the CORE voltage. */
> > + if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
> > + return;
> > + }
> > +
> > + /* Set MPU Frequency to what we detected now that voltages are set */
> > + mpu_pll_config_val(mpu_pll);
> > +}
> > #endif
> >
>
> Would be possible rework this function in order to be more generic ?
> Or maybe split the parts that are common from the parts that are board
> specific ?
>
> I think part of this code can be shared for other boards that use
> AM335x processor ( like IGEP COM AQUILA.)
I think the answer is probably, depending on what other boards need.
The code to read efuse_sma is generic. The code to poke the tps65217 is
specific to tps65217-using boards but the sequence (aside from the
board_is_bone checks) would be generic. Same with tps65910 using
boards. Maybe the probe-and-check logic could be factored back to the
pmic file. I'll take a gander..
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130719/bc493bcc/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-19 19:48 ` Tom Rini
@ 2013-07-19 20:58 ` Tom Rini
0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-07-19 20:58 UTC (permalink / raw)
To: u-boot
On Fri, Jul 19, 2013 at 03:48:44PM -0400, Tom Rini wrote:
> On Fri, Jul 19, 2013 at 09:37:51PM +0200, Enric Balletbo Serra wrote:
> > Hi Tom,
> >
> > Just a comment below ...
> >
> > 2013/7/19 Tom Rini <trini@ti.com>:
> > > Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
> > > depending on the board we are running on. In all cases, we see if we
> > > can rely on the efuse_sma register to tell us the maximum speed. In the
> > > case of Beaglebone White, we need to make sure we are on AC power, and
> > > are on later than rev A1, and then we can ramp up to the PG1.0 maximum
> > > of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
> > > supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
> > > cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
> > > need to rely on the efuse_sma if we are on PG2.1, and the defaults for
> > > PG1.0/2.0.
> > >
> > > Signed-off-by: Tom Rini <trini@ti.com>
> > > ---
> > > arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
> > > board/ti/am335x/board.c | 155 ++++++++++++++++++++++
> > > include/configs/am335x_evm.h | 4 +
> > > 3 files changed, 167 insertions(+)
> > >
> > > diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > > index 89b63d9..834f24f 100644
> > > --- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > > +++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> > > @@ -24,6 +24,14 @@
> > > #define CONFIG_SYS_MPUCLK 550
> > > #endif
> > >
> > > +/* MAIN PLL Fdll supported frequencies */
> > > +#define MPUPLL_M_1000 1000
> > > +#define MPUPLL_M_800 800
> > > +#define MPUPLL_M_720 720
> > > +#define MPUPLL_M_600 600
> > > +#define MPUPLL_M_550 550
> > > +#define MPUPLL_M_300 300
> > > +
> > > extern void pll_init(void);
> > > extern void enable_emif_clocks(void);
> > > extern void enable_dmm_clocks(void);
> > > diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> > > index fdbe26c..6544931 100644
> > > --- a/board/ti/am335x/board.c
> > > +++ b/board/ti/am335x/board.c
> > > @@ -33,6 +33,8 @@
> > > #include <i2c.h>
> > > #include <miiphy.h>
> > > #include <cpsw.h>
> > > +#include <power/tps65217.h>
> > > +#include <power/tps65910.h>
> > > #include "board.h"
> > >
> > > DECLARE_GLOBAL_DATA_PTR;
> > > @@ -282,6 +284,159 @@ int spl_start_uboot(void)
> > > }
> > > #endif
> > >
> > > +void am33xx_spl_board_init(void)
> > > +{
> > > + int mpu_vdd, mpu_pll, sil_rev;
> > > +
> > > + /* Assume PG 1.0 */
> > > + mpu_pll = MPUPLL_M_720;
> > > +
> > > + sil_rev = readl(&cdev->deviceid) >> 28;
> > > + if (sil_rev == 1)
> > > + /* PG 2.0, efuse may not be set. */
> > > + mpu_pll = MPUPLL_M_800;
> > > + else if (sil_rev >= 2) {
> > > + /* Check what the efuse says our max speed is. */
> > > + int efuse_arm_mpu_max_freq;
> > > + efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
> > > + switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
> > > + case AM335X_ZCZ_1000:
> > > + mpu_pll = MPUPLL_M_1000;
> > > + break;
> > > + case AM335X_ZCZ_800:
> > > + mpu_pll = MPUPLL_M_800;
> > > + break;
> > > + case AM335X_ZCZ_720:
> > > + mpu_pll = MPUPLL_M_720;
> > > + break;
> > > + case AM335X_ZCZ_600:
> > > + case AM335X_ZCE_600:
> > > + mpu_pll = MPUPLL_M_600;
> > > + break;
> > > + case AM335X_ZCZ_300:
> > > + case AM335X_ZCE_300:
> > > + mpu_pll = MPUPLL_M_300;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (board_is_bone() || board_is_bone_lt()) {
> > > + /* BeagleBone PMIC Code */
> > > + int usb_cur_lim;
> > > +
> > > + /*
> > > + * Only perform PMIC configurations if board rev > A1
> > > + * on Beaglebone White
> > > + */
> > > + if (board_is_bone() && !strncmp(header.version, "00A1", 4))
> > > + return;
> > > +
> > > + if (i2c_probe(TPS65217_CHIP_PM))
> > > + return;
> > > +
> > > + /*
> > > + * On Beaglebone White we need to ensure we have AC power
> > > + * before increasing the frequency.
> > > + */
> > > + if (board_is_bone()) {
> > > + uchar pmic_status_reg;
> > > + if (tps65217_reg_read(STATUS, &pmic_status_reg))
> > > + return;
> > > + if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
> > > + puts("No AC power, disabling frequency switch\n");
> > > + return;
> > > + }
> > > + }
> > > +
> > > + /*
> > > + * Increase USB current limit to 1300mA or 1800mA and set
> > > + * the MPU voltage controller as needed.
> > > + */
> > > + if (mpu_pll == MPUPLL_M_1000) {
> > > + usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
> > > + mpu_vdd = DCDC_VOLT_SEL_1325MV;
> > > + } else {
> > > + usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
> > > + mpu_vdd = DCDC_VOLT_SEL_1275MV;
> > > + }
> > > +
> > > + if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
> > > + usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
> > > + printf("tps65217_reg_write failure\n");
> > > +
> > > +
> > > + /* Set DCDC2 (MPU) voltage */
> > > + if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
> > > + printf("tps65217_voltage_update failure\n");
> > > + return;
> > > + }
> > > +
> > > + /* Set LDO3, LDO4 output voltage to 3.3V */
> > > + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
> > > + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> > > + printf("tps65217_reg_write failure\n");
> > > +
> > > + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
> > > + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> > > + printf("tps65217_reg_write failure\n");
> > > + } else {
> > > + uchar buf[4];
> > > +
> > > + /*
> > > + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
> > > + * MPU frequencies we support we use a CORE voltage of
> > > + * 1.1375V. For MPU voltage we need to switch based on
> > > + * the frequency we are running at.
> > > + */
> > > + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
> > > + return;
> > > +
> > > + /* VDD1/2 voltage selection register access by control i/f */
> > > + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> > > + buf, 1))
> > > + return;
> > > +
> > > + buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
> > > +
> > > + if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> > > + buf, 1))
> > > + return;
> > > +
> > > + /* Depending on MPU clock we need different MPU VDD */
> > > +
> > > + /* Default to PG1.0/PG2.0 values. */
> > > + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> > > +
> > > + if (sil_rev >= 2) {
> > > + switch (mpu_pll) {
> > > + case MPUPLL_M_1000:
> > > + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
> > > + break;
> > > + case MPUPLL_M_800:
> > > + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
> > > + break;
> > > + case MPUPLL_M_720:
> > > + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
> > > + break;
> > > + case MPUPLL_M_600:
> > > + case MPUPLL_M_300:
> > > + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + /* First update MPU voltage. */
> > > + if (tps65910_voltage_update(MPU, mpu_vdd))
> > > + return;
> > > +
> > > + /* Second, update the CORE voltage. */
> > > + if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
> > > + return;
> > > + }
> > > +
> > > + /* Set MPU Frequency to what we detected now that voltages are set */
> > > + mpu_pll_config_val(mpu_pll);
> > > +}
> > > #endif
> > >
> >
> > Would be possible rework this function in order to be more generic ?
> > Or maybe split the parts that are common from the parts that are board
> > specific ?
> >
> > I think part of this code can be shared for other boards that use
> > AM335x processor ( like IGEP COM AQUILA.)
>
> I think the answer is probably, depending on what other boards need.
> The code to read efuse_sma is generic. The code to poke the tps65217 is
> specific to tps65217-using boards but the sequence (aside from the
> board_is_bone checks) would be generic. Same with tps65910 using
> boards. Maybe the probe-and-check logic could be factored back to the
> pmic file. I'll take a gander..
On further investigation, and whacking at why Beaglebone Black wasn't
working quite right with this patch. So, it's already about as generic
as it can be, at least without further examples to work off of.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130719/f6cbcb4f/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-19 19:00 ` [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency Tom Rini
2013-07-19 19:37 ` Enric Balletbo Serra
@ 2013-07-23 18:46 ` Dan Murphy
2013-07-29 15:57 ` Enric Balletbo Serra
1 sibling, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2013-07-23 18:46 UTC (permalink / raw)
To: u-boot
On 07/19/2013 02:00 PM, Tom Rini wrote:
> Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
> depending on the board we are running on. In all cases, we see if we
> can rely on the efuse_sma register to tell us the maximum speed. In the
> case of Beaglebone White, we need to make sure we are on AC power, and
> are on later than rev A1, and then we can ramp up to the PG1.0 maximum
> of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
> supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
> cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
> need to rely on the efuse_sma if we are on PG2.1, and the defaults for
> PG1.0/2.0.
>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
> board/ti/am335x/board.c | 155 ++++++++++++++++++++++
> include/configs/am335x_evm.h | 4 +
> 3 files changed, 167 insertions(+)
>
> diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> index 89b63d9..834f24f 100644
> --- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> +++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
> @@ -24,6 +24,14 @@
> #define CONFIG_SYS_MPUCLK 550
> #endif
>
> +/* MAIN PLL Fdll supported frequencies */
> +#define MPUPLL_M_1000 1000
> +#define MPUPLL_M_800 800
> +#define MPUPLL_M_720 720
> +#define MPUPLL_M_600 600
> +#define MPUPLL_M_550 550
> +#define MPUPLL_M_300 300
> +
> extern void pll_init(void);
> extern void enable_emif_clocks(void);
> extern void enable_dmm_clocks(void);
> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> index fdbe26c..6544931 100644
> --- a/board/ti/am335x/board.c
> +++ b/board/ti/am335x/board.c
> @@ -33,6 +33,8 @@
> #include <i2c.h>
> #include <miiphy.h>
> #include <cpsw.h>
> +#include <power/tps65217.h>
> +#include <power/tps65910.h>
> #include "board.h"
>
> DECLARE_GLOBAL_DATA_PTR;
> @@ -282,6 +284,159 @@ int spl_start_uboot(void)
> }
> #endif
>
> +void am33xx_spl_board_init(void)
> +{
> + int mpu_vdd, mpu_pll, sil_rev;
> +
> + /* Assume PG 1.0 */
> + mpu_pll = MPUPLL_M_720;
> +
> + sil_rev = readl(&cdev->deviceid) >> 28;
> + if (sil_rev == 1)
> + /* PG 2.0, efuse may not be set. */
> + mpu_pll = MPUPLL_M_800;
> + else if (sil_rev >= 2) {
> + /* Check what the efuse says our max speed is. */
> + int efuse_arm_mpu_max_freq;
> + efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
> + switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
> + case AM335X_ZCZ_1000:
> + mpu_pll = MPUPLL_M_1000;
> + break;
> + case AM335X_ZCZ_800:
> + mpu_pll = MPUPLL_M_800;
> + break;
> + case AM335X_ZCZ_720:
> + mpu_pll = MPUPLL_M_720;
> + break;
> + case AM335X_ZCZ_600:
> + case AM335X_ZCE_600:
> + mpu_pll = MPUPLL_M_600;
> + break;
> + case AM335X_ZCZ_300:
> + case AM335X_ZCE_300:
> + mpu_pll = MPUPLL_M_300;
> + break;
> + }
> + }
> +
> + if (board_is_bone() || board_is_bone_lt()) {
> + /* BeagleBone PMIC Code */
> + int usb_cur_lim;
> +
> + /*
> + * Only perform PMIC configurations if board rev > A1
> + * on Beaglebone White
> + */
> + if (board_is_bone() && !strncmp(header.version, "00A1", 4))
> + return;
> +
> + if (i2c_probe(TPS65217_CHIP_PM))
> + return;
> +
> + /*
> + * On Beaglebone White we need to ensure we have AC power
> + * before increasing the frequency.
> + */
> + if (board_is_bone()) {
> + uchar pmic_status_reg;
> + if (tps65217_reg_read(STATUS, &pmic_status_reg))
> + return;
> + if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
> + puts("No AC power, disabling frequency switch\n");
> + return;
> + }
> + }
> +
> + /*
> + * Increase USB current limit to 1300mA or 1800mA and set
> + * the MPU voltage controller as needed.
> + */
> + if (mpu_pll == MPUPLL_M_1000) {
> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
> + mpu_vdd = DCDC_VOLT_SEL_1325MV;
> + } else {
> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
> + mpu_vdd = DCDC_VOLT_SEL_1275MV;
> + }
> +
> + if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
> + usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
> + printf("tps65217_reg_write failure\n");
puts("...") when we don't have format characters, thanks.
> +
> +
> + /* Set DCDC2 (MPU) voltage */
> + if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
> + printf("tps65217_voltage_update failure\n");
Same as above
> + return;
> + }
> +
> + /* Set LDO3, LDO4 output voltage to 3.3V */
> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> + printf("tps65217_reg_write failure\n");
Same as above
> +
> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
> + printf("tps65217_reg_write failure\n");
Same as above
> + } else {
> + uchar buf[4];
> +
> + /*
> + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
> + * MPU frequencies we support we use a CORE voltage of
> + * 1.1375V. For MPU voltage we need to switch based on
> + * the frequency we are running at.
> + */
> + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
> + return;
> +
> + /* VDD1/2 voltage selection register access by control i/f */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> + buf, 1))
Would it not be better to have an API in the pmic driver file that you can call to access the pmic
instead of calling the pmic explicitly?
> + return;
> +
> + buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> + buf, 1))
Would it not be better to have an API in the pmic driver file that you can call to access the pmic
instead of calling the pmic explicitly?
> + return;
> +
> + /* Depending on MPU clock we need different MPU VDD */
> +
> + /* Default to PG1.0/PG2.0 values. */
> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> +
> + if (sil_rev >= 2) {
> + switch (mpu_pll) {
> + case MPUPLL_M_1000:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
> + break;
> + case MPUPLL_M_800:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
> + break;
> + case MPUPLL_M_720:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
> + break;
> + case MPUPLL_M_600:
> + case MPUPLL_M_300:
> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> + break;
> + }
> + }
> +
> + /* First update MPU voltage. */
> + if (tps65910_voltage_update(MPU, mpu_vdd))
> + return;
> +
> + /* Second, update the CORE voltage. */
> + if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
> + return;
> + }
> +
> + /* Set MPU Frequency to what we detected now that voltages are set */
> + mpu_pll_config_val(mpu_pll);
> +}
> #endif
>
> /*
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index c5a6d4b..60c87a3 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -289,6 +289,9 @@
> #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
> #define CONFIG_SYS_I2C_MULTI_EEPROMS
>
> +#define CONFIG_POWER_TPS65217
> +#define CONFIG_POWER_TPS65910
> +
> #define CONFIG_OMAP_GPIO
>
> #define CONFIG_BAUDRATE 115200
> @@ -352,6 +355,7 @@
> #define CONFIG_SPL_LIBCOMMON_SUPPORT
> #define CONFIG_SPL_LIBDISK_SUPPORT
> #define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_POWER_SUPPORT
> #define CONFIG_SPL_SERIAL_SUPPORT
> #define CONFIG_SPL_GPIO_SUPPORT
> #define CONFIG_SPL_YMODEM_SUPPORT
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-23 18:46 ` Dan Murphy
@ 2013-07-29 15:57 ` Enric Balletbo Serra
2013-08-07 16:20 ` Tom Rini
2013-08-07 16:56 ` Tom Rini
0 siblings, 2 replies; 21+ messages in thread
From: Enric Balletbo Serra @ 2013-07-29 15:57 UTC (permalink / raw)
To: u-boot
Hi Tom,
2013/7/23 Dan Murphy <dmurphy@ti.com>:
> On 07/19/2013 02:00 PM, Tom Rini wrote:
>> Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
>> depending on the board we are running on. In all cases, we see if we
>> can rely on the efuse_sma register to tell us the maximum speed. In the
>> case of Beaglebone White, we need to make sure we are on AC power, and
>> are on later than rev A1, and then we can ramp up to the PG1.0 maximum
>> of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
>> supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
>> cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
>> need to rely on the efuse_sma if we are on PG2.1, and the defaults for
>> PG1.0/2.0.
>>
>> Signed-off-by: Tom Rini <trini@ti.com>
>> ---
>> arch/arm/include/asm/arch-am33xx/clocks_am33xx.h | 8 ++
>> board/ti/am335x/board.c | 155 ++++++++++++++++++++++
>> include/configs/am335x_evm.h | 4 +
>> 3 files changed, 167 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
>> index 89b63d9..834f24f 100644
>> --- a/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
>> +++ b/arch/arm/include/asm/arch-am33xx/clocks_am33xx.h
>> @@ -24,6 +24,14 @@
>> #define CONFIG_SYS_MPUCLK 550
>> #endif
>>
>> +/* MAIN PLL Fdll supported frequencies */
>> +#define MPUPLL_M_1000 1000
>> +#define MPUPLL_M_800 800
>> +#define MPUPLL_M_720 720
>> +#define MPUPLL_M_600 600
>> +#define MPUPLL_M_550 550
>> +#define MPUPLL_M_300 300
>> +
>> extern void pll_init(void);
>> extern void enable_emif_clocks(void);
>> extern void enable_dmm_clocks(void);
>> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
>> index fdbe26c..6544931 100644
>> --- a/board/ti/am335x/board.c
>> +++ b/board/ti/am335x/board.c
>> @@ -33,6 +33,8 @@
>> #include <i2c.h>
>> #include <miiphy.h>
>> #include <cpsw.h>
>> +#include <power/tps65217.h>
>> +#include <power/tps65910.h>
>> #include "board.h"
>>
>> DECLARE_GLOBAL_DATA_PTR;
>> @@ -282,6 +284,159 @@ int spl_start_uboot(void)
>> }
>> #endif
>>
>> +void am33xx_spl_board_init(void)
>> +{
>> + int mpu_vdd, mpu_pll, sil_rev;
>> +
>> + /* Assume PG 1.0 */
>> + mpu_pll = MPUPLL_M_720;
>> +
>> + sil_rev = readl(&cdev->deviceid) >> 28;
>> + if (sil_rev == 1)
>> + /* PG 2.0, efuse may not be set. */
>> + mpu_pll = MPUPLL_M_800;
>> + else if (sil_rev >= 2) {
>> + /* Check what the efuse says our max speed is. */
>> + int efuse_arm_mpu_max_freq;
>> + efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
>> + switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
>> + case AM335X_ZCZ_1000:
>> + mpu_pll = MPUPLL_M_1000;
>> + break;
>> + case AM335X_ZCZ_800:
>> + mpu_pll = MPUPLL_M_800;
>> + break;
>> + case AM335X_ZCZ_720:
>> + mpu_pll = MPUPLL_M_720;
>> + break;
>> + case AM335X_ZCZ_600:
>> + case AM335X_ZCE_600:
>> + mpu_pll = MPUPLL_M_600;
>> + break;
>> + case AM335X_ZCZ_300:
>> + case AM335X_ZCE_300:
>> + mpu_pll = MPUPLL_M_300;
>> + break;
>> + }
>> + }
>> +
>> + if (board_is_bone() || board_is_bone_lt()) {
>> + /* BeagleBone PMIC Code */
>> + int usb_cur_lim;
>> +
>> + /*
>> + * Only perform PMIC configurations if board rev > A1
>> + * on Beaglebone White
>> + */
>> + if (board_is_bone() && !strncmp(header.version, "00A1", 4))
>> + return;
>> +
>> + if (i2c_probe(TPS65217_CHIP_PM))
>> + return;
>> +
>> + /*
>> + * On Beaglebone White we need to ensure we have AC power
>> + * before increasing the frequency.
>> + */
>> + if (board_is_bone()) {
>> + uchar pmic_status_reg;
>> + if (tps65217_reg_read(STATUS, &pmic_status_reg))
>> + return;
>> + if (!(pmic_status_reg & PWR_SRC_AC_BITMASK)) {
>> + puts("No AC power, disabling frequency switch\n");
>> + return;
>> + }
>> + }
>> +
>> + /*
>> + * Increase USB current limit to 1300mA or 1800mA and set
>> + * the MPU voltage controller as needed.
>> + */
>> + if (mpu_pll == MPUPLL_M_1000) {
>> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1800MA;
>> + mpu_vdd = DCDC_VOLT_SEL_1325MV;
>> + } else {
>> + usb_cur_lim = USB_INPUT_CUR_LIMIT_1300MA;
>> + mpu_vdd = DCDC_VOLT_SEL_1275MV;
>> + }
>> +
>> + if (tps65217_reg_write(PROT_LEVEL_NONE, POWER_PATH,
>> + usb_cur_lim, USB_INPUT_CUR_LIMIT_MASK))
>> + printf("tps65217_reg_write failure\n");
>
> puts("...") when we don't have format characters, thanks.
>
>> +
>> +
>> + /* Set DCDC2 (MPU) voltage */
>> + if (tps65217_voltage_update(DEFDCDC2, mpu_vdd)) {
>> + printf("tps65217_voltage_update failure\n");
>
> Same as above
>
>
>> + return;
>> + }
>> +
>> + /* Set LDO3, LDO4 output voltage to 3.3V */
>> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS1,
>> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
>> + printf("tps65217_reg_write failure\n");
>
> Same as above
>
>
>> +
>> + if (tps65217_reg_write(PROT_LEVEL_2, DEFLS2,
>> + LDO_VOLTAGE_OUT_3_3, LDO_MASK))
>> + printf("tps65217_reg_write failure\n");
>
> Same as above
>
>
>> + } else {
>> + uchar buf[4];
>> +
>> + /*
>> + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
>> + * MPU frequencies we support we use a CORE voltage of
>> + * 1.1375V. For MPU voltage we need to switch based on
>> + * the frequency we are running at.
>> + */
>> + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
>> + return;
>> +
>> + /* VDD1/2 voltage selection register access by control i/f */
>> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
>> + buf, 1))
>
> Would it not be better to have an API in the pmic driver file that you can call to access the pmic
> instead of calling the pmic explicitly?
>
>> + return;
>> +
>> + buf[0] |= TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C;
>> +
>> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
>> + buf, 1))
>
> Would it not be better to have an API in the pmic driver file that you can call to access the pmic
> instead of calling the pmic explicitly?
>
>> + return;
>> +
>> + /* Depending on MPU clock we need different MPU VDD */
>> +
>> + /* Default to PG1.0/PG2.0 values. */
>> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
>> +
>> + if (sil_rev >= 2) {
>> + switch (mpu_pll) {
>> + case MPUPLL_M_1000:
>> + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
>> + break;
>> + case MPUPLL_M_800:
>> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
>> + break;
>> + case MPUPLL_M_720:
>> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
>> + break;
>> + case MPUPLL_M_600:
>> + case MPUPLL_M_300:
>> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
>> + break;
>> + }
>> + }
>> +
>> + /* First update MPU voltage. */
>> + if (tps65910_voltage_update(MPU, mpu_vdd))
>> + return;
>> +
>> + /* Second, update the CORE voltage. */
>> + if (tps65910_voltage_update(CORE, TPS65910_OP_REG_SEL_1_1_3))
>> + return;
>> + }
>> +
>> + /* Set MPU Frequency to what we detected now that voltages are set */
>> + mpu_pll_config_val(mpu_pll);
>> +}
As example, I just pushed[1] in my personal git an implementation of
am33xx_spl_board_init for IGEP COM AQUILA AM335x based on your
patches. As you can see this function is very similar to the function
implemented in your patch. I wonder if it's possible implement a more
generic form in order to avoid repeating code in board files. Maybe
something like this :
For am335x_evm :
am33xx_spl_board_init {
/* Get MPU max frequency */
mpu_pll = am335x_get_efuse_mpu_max_freq()
if (board_is_bone() || board_is_bone_lt())
tps65217_set_mpu_voltagel(mpu_pll) /* not sure if this can be generic */
else
tp65910_set_mpu_voltagel(mpu_pll)
/* Set MPU Frequency to what we detected now that voltages are set */
mpu_pll_config_val(mpu_pll);
}
For IGEP COM AQUILA:
am33xx_spl_board_init {
/* Get MPU max frequency */
mpu_pll = am335x_get_efuse_mpu_max_freq()
/* Set PMIC MPU voltage */
tp65910_set_mpu_voltagel(mpu_pll)
/* Set MPU Frequency to what we detected now that voltages are set */
mpu_pll_config_val(mpu_pll);
}
[1] https://github.com/eballetbo/u-boot/commit/2f5b68bd9b6978d7571c846d7ce8d183ead571b1
>> #endif
>>
>> /*
>> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
>> index c5a6d4b..60c87a3 100644
>> --- a/include/configs/am335x_evm.h
>> +++ b/include/configs/am335x_evm.h
>> @@ -289,6 +289,9 @@
>> #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
>> #define CONFIG_SYS_I2C_MULTI_EEPROMS
>>
>> +#define CONFIG_POWER_TPS65217
>> +#define CONFIG_POWER_TPS65910
>> +
>> #define CONFIG_OMAP_GPIO
>>
>> #define CONFIG_BAUDRATE 115200
>> @@ -352,6 +355,7 @@
>> #define CONFIG_SPL_LIBCOMMON_SUPPORT
>> #define CONFIG_SPL_LIBDISK_SUPPORT
>> #define CONFIG_SPL_LIBGENERIC_SUPPORT
>> +#define CONFIG_SPL_POWER_SUPPORT
>> #define CONFIG_SPL_SERIAL_SUPPORT
>> #define CONFIG_SPL_GPIO_SUPPORT
>> #define CONFIG_SPL_YMODEM_SUPPORT
>
>
> --
> ------------------
> Dan Murphy
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Best Regards,
Enric
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-29 15:57 ` Enric Balletbo Serra
@ 2013-08-07 16:20 ` Tom Rini
2013-08-07 16:56 ` Tom Rini
1 sibling, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-08-07 16:20 UTC (permalink / raw)
To: u-boot
On Mon, Jul 29, 2013 at 05:57:22PM +0200, Enric Balletbo Serra wrote:
> Hi Tom,
>
> 2013/7/23 Dan Murphy <dmurphy@ti.com>:
> > On 07/19/2013 02:00 PM, Tom Rini wrote:
> >> Add a am33xx_spl_board_init (and enable the PMICs) that we may see,
> >> depending on the board we are running on. In all cases, we see if we
> >> can rely on the efuse_sma register to tell us the maximum speed. In the
> >> case of Beaglebone White, we need to make sure we are on AC power, and
> >> are on later than rev A1, and then we can ramp up to the PG1.0 maximum
> >> of 720Mhz. In the case of Beaglebone Black, we are either on PG2.0 that
> >> supports 1GHz or PG2.1. As PG2.0 may or may not have efuse_sma set, we
> >> cannot rely on this probe. In the case of the GP EVM, EVM SK and IDK we
> >> need to rely on the efuse_sma if we are on PG2.1, and the defaults for
> >> PG1.0/2.0.
[snip]
> >> + /*
> >> + * The GP EVM, IDK and EVM SK use a TPS65910 PMIC. For all
> >> + * MPU frequencies we support we use a CORE voltage of
> >> + * 1.1375V. For MPU voltage we need to switch based on
> >> + * the frequency we are running at.
> >> + */
> >> + if (i2c_probe(TPS65910_CTRL_I2C_ADDR))
> >> + return;
> >> +
> >> + /* VDD1/2 voltage selection register access by control i/f */
> >> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, TPS65910_DEVCTRL_REG, 1,
> >> + buf, 1))
> >
> > Would it not be better to have an API in the pmic driver file that
> > you can call to access the pmic instead of calling the pmic
> > explicitly?
So, somewhat.
[snip]
> >> + /* Depending on MPU clock we need different MPU VDD */
> >> +
> >> + /* Default to PG1.0/PG2.0 values. */
> >> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> >> +
> >> + if (sil_rev >= 2) {
> >> + switch (mpu_pll) {
> >> + case MPUPLL_M_1000:
> >> + mpu_vdd = TPS65910_OP_REG_SEL_1_3_2_5;
> >> + break;
> >> + case MPUPLL_M_800:
> >> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_6;
> >> + break;
> >> + case MPUPLL_M_720:
> >> + mpu_vdd = TPS65910_OP_REG_SEL_1_2_0;
> >> + break;
> >> + case MPUPLL_M_600:
> >> + case MPUPLL_M_300:
> >> + mpu_vdd = TPS65910_OP_REG_SEL_1_1_3;
> >> + break;
> >> + }
> >> + }
[snip]
> As example, I just pushed[1] in my personal git an implementation of
> am33xx_spl_board_init for IGEP COM AQUILA AM335x based on your
> patches. As you can see this function is very similar to the function
> implemented in your patch. I wonder if it's possible implement a more
> generic form in order to avoid repeating code in board files. Maybe
> something like this :
>
> For am335x_evm :
>
> am33xx_spl_board_init {
> /* Get MPU max frequency */
> mpu_pll = am335x_get_efuse_mpu_max_freq()
> if (board_is_bone() || board_is_bone_lt())
> tps65217_set_mpu_voltagel(mpu_pll) /* not sure if this can be generic */
> else
> tp65910_set_mpu_voltagel(mpu_pll)
The problem is that the TPS65910 family supports a wide range of non-TI
platforms (s5p, rockchip rk29xx, i.mx) so we can't hide the PG A.B ->
voltage required bit in the tps65910 call. I'll see how something looks
in arch/arm/cpu/armv7/am33xx/sys_info.c along with the
efuse_mpu_max_freq function.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130807/5e8b93d1/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread* [U-Boot] [PATCH 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
2013-07-29 15:57 ` Enric Balletbo Serra
2013-08-07 16:20 ` Tom Rini
@ 2013-08-07 16:56 ` Tom Rini
1 sibling, 0 replies; 21+ messages in thread
From: Tom Rini @ 2013-08-07 16:56 UTC (permalink / raw)
To: u-boot
On Mon, Jul 29, 2013 at 05:57:22PM +0200, Enric Balletbo Serra wrote:
> Hi Tom,
[snip]
> As example, I just pushed[1] in my personal git an implementation of
> am33xx_spl_board_init for IGEP COM AQUILA AM335x based on your
> patches. As you can see this function is very similar to the function
> implemented in your patch. I wonder if it's possible implement a more
> generic form in order to avoid repeating code in board files. Maybe
> something like this :
>
> For am335x_evm :
>
> am33xx_spl_board_init {
> /* Get MPU max frequency */
> mpu_pll = am335x_get_efuse_mpu_max_freq()
> if (board_is_bone() || board_is_bone_lt())
> tps65217_set_mpu_voltagel(mpu_pll) /* not sure if this can be generic */
> else
> tp65910_set_mpu_voltagel(mpu_pll)
>
> /* Set MPU Frequency to what we detected now that voltages are set */
> mpu_pll_config_val(mpu_pll);
> }
>
> For IGEP COM AQUILA:
>
> am33xx_spl_board_init {
> /* Get MPU max frequency */
> mpu_pll = am335x_get_efuse_mpu_max_freq()
>
> /* Set PMIC MPU voltage */
> tp65910_set_mpu_voltagel(mpu_pll)
>
> /* Set MPU Frequency to what we detected now that voltages are set */
> mpu_pll_config_val(mpu_pll);
> }
>
> [1] https://github.com/eballetbo/u-boot/commit/2f5b68bd9b6978d7571c846d7ce8d183ead571b1
OK, I've done some consolidation, but the per-board part is still a few
calls since as I noted in my other reply, tps65910 is not a
am335x-specific chip. Heiko, can you take a quick look at how the
siemens boards would use this? Are they also using the tps65910 or
something else? I'm not opposed to throwing a
am335x_tps65910_set_max_mpu() or so in sys_info.c or something and
marking it weak just in case another way of using the 65910 ends up
existing, but I don't want to throw it into
drivers/power/pmic/pmic_tps65910.c. Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130807/293737bf/attachment.pgp>
^ permalink raw reply [flat|nested] 21+ messages in thread