Re: [U-Boot] [PATCH] Added MC13892VK Power Management driver
Hi Tom,
Tom wrote:
Stefano Babic wrote:
Added SPI driver for the Power Management Controller
used with i.MX51 Processor.
Errors in
imx31_litekeit
imx31_phycore_eet
imx31_ads
These must be fixed
...and I have to test with a MX31, too. I will find a board and test the
changes before sending version 2 of the patch.
+
+tmp = pmic_tx;
+for (i = 0; i 4; i++) {
+tx[i] = (tmp 0xFF00) 24;
+tmp = 8;
+}
Looks like you are converting for bigendian to little.
It may be better to use one of the cpu_to_little functions
see linux/byteorder/generic.h
Thanks, I will check it.
+pmic_rx = (rx[0] 24) | (rx[1] 16) | (rx[2] 8) | rx[3];
A conversion of little to big.
Use existing conversion functions
ok, thanks.
+rev_id = mc13892_reg(slave, 7, 0, 0);
Be consistent on use of mc13892_reg.
If you have the read/write wrappers, use them.
From this conext it is difficult to understand that this is a read.
Agree, this is a mistake, I forget to correct it, mc13892_reg() must not
be used outside the driver.
The return is a u32 value, not a pointer so the modifier 'volatile' is
not needed in the declaration of rev_id
Ok, got it.
+MC13892_REG_LED_CTL2,
+MC13892_REG_LED_CTL3,
+MC13892_REG_UNUSED12,
+MC13892_REG_UNUSED13,
+MC13892_REG_TRIM0,
+MC13892_REG_TRIM1,
+MC13892_REG_TEST0,
+MC13892_REG_TEST1,/*60 */
+MC13892_REG_TEST2,
+MC13892_REG_TEST3,
+MC13892_REG_TEST4,
This should be converted to the reg access through struct members.
I do not know if this helps to make code more readable. We need to send
the address of the internal register via the SPI interface, and these
registers are not mapped into the microprocessor address space.
If I use a structure, I must always convert the address to the register
number before sending via the SPI interface. At the end, I have a
further step that probably makes difficult to understand.
The same approach with register number is actually used for
drivers/rtc/mc13873-rtc.c, whose interface is quite similar as this one,
even if in this case the register number is directly used. I think it is
better to have the same approach for similar chips.
These wrappers add an unneeded layer of indirection
There should be real read / write funtions
Agree, I will remove that indirection and avoid to export mc13892_reg.
Stefano
--
=
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added MC13892VK Power Management driver
Stefano Babic wrote:
Added SPI driver for the Power Management Controller
used with i.MX51 Processor.
Errors in
imx31_litekeit
imx31_phycore_eet
imx31_ads
imx31_pdk*
mxc_spi.c: In function 'spi_cfg':
mxc_spi.c:158: warning: implicit declaration of function ' mxc_get_clock'
mxc_spi.c:158: error: ' MXC_CSPI_CLK' undeclared (first use in this function)
mxc_spi.c:158: error: (Each undeclared identifier is reported only once
mxc_spi.c:158: error: for each function it appears in.)
mxc_spi.c:197: warning: implicit declaration of function '
MXC_CSPICTRL_SELCHAN'
mxc_spi.c:199: warning: implicit declaration of function '
MXC_CSPICTRL_PREDIV'
mxc_spi.c:201: warning: implicit declaration of function '
MXC_CSPICTRL_POSTDIV'
mxc_spi.c:219: error: ' MXC_CSPICON' undeclared (first use in this function)
mxc_spi.c:225: error: ' MXC_CSPICON_SSPOL' undeclared (first use in this
function)
mxc_spi.c:227: error: ' MXC_CSPICON_POL' undeclared (first use in this
function)
mxc_spi.c:229: error: ' MXC_CSPICON_PHA' undeclared (first use in this
function)
mxc_spi.c: In function 'spi_setup_slave':
mxc_spi.c:460: error: 'ctrl_reg' undeclared (first use in this function)
imx51evk
mxc_spi.c: In function 'decode_cs':
mxc_spi.c:400: warning: unused variable 'ret'
These must be fixed
Signed-off-by: Stefano Babic sba...@denx.de
---
drivers/misc/Makefile |1 +
drivers/misc/mc13892_spi_pmic.c | 134 +++
include/mc13892.h | 149
+++
3 files changed, 284 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/mc13892_spi_pmic.c
create mode 100644 include/mc13892.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index f6df60f..5847262 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -31,6 +31,7 @@ COBJS-$(CONFIG_FSL_LAW) += fsl_law.o
COBJS-$(CONFIG_NS87308) += ns87308.o
COBJS-$(CONFIG_STATUS_LED) += status_led.o
COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_MC13892_SPI_PMIC) += mc13892_spi_pmic.o
COBJS:= $(COBJS-y)
SRCS := $(COBJS:.o=.c)
diff --git a/drivers/misc/mc13892_spi_pmic.c b/drivers/misc/mc13892_spi_pmic.c
new file mode 100644
index 000..f095fcc
--- /dev/null
+++ b/drivers/misc/mc13892_spi_pmic.c
@@ -0,0 +1,134 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+ *
+ * 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 config.h
+#include common.h
+#include spi.h
+#include asm/errno.h
+#include linux/types.h
+
+u32 mc13892_reg(struct spi_slave *slave, u32 reg, u32 val, u32 write)
+{
+ char tx[4];
+ char rx[4];
+ int i;
+ u32 tmp, pmic_tx, pmic_rx;
+
+ if (!slave)
+ return 0;
+
+ if (reg 63 || write 1) {
+ printf(reg num = %d is invalid. Should be less then 63\n,
+ reg);
+ return 0;
+ }
+ pmic_tx = (write 31) | (reg 25) | (val 0x00FF);
+ debug(reg=0x%x, val=0x%08x\n, reg, pmic_tx);
+
+ tmp = pmic_tx;
+ for (i = 0; i 4; i++) {
+ tx[i] = (tmp 0xFF00) 24;
+ tmp = 8;
+ }
Looks like you are converting for bigendian to little.
It may be better to use one of the cpu_to_little functions
see linux/byteorder/generic.h
+
+ if (spi_xfer(slave, 4 3, tx, rx,
+ SPI_XFER_BEGIN | SPI_XFER_END)) {
+ return -1;
+ }
+
+ if (write) {
+ tx[0] = ~(1 31);
+ if (spi_xfer(slave, 4 3, tx, rx,
+ SPI_XFER_BEGIN | SPI_XFER_END)) {
+ return -1;
+ }
+ }
+
+ pmic_rx = (rx[0] 24) | (rx[1] 16) | (rx[2] 8) | rx[3];
A conversion of little to big.
Use existing conversion functions
+ debug(reg=0x%x, val_read=0x%08x 0x%x 0x%x 0x%x 0x%x\n,
+ reg, pmic_rx, rx[0], rx[1], rx[2], rx[3]);
+ return pmic_rx;
+}
+
+void mc13892_show_pmic_info(struct spi_slave *slave)
+{
+ volatile u32 rev_id;
+
+ if (!slave)
+ return;
+
+ rev_id = mc13892_reg(slave, 7, 0, 0);
Be consistent on
[U-Boot] [PATCH] Added MC13892VK Power Management driver
Added SPI driver for the Power Management Controller
used with i.MX51 Processor.
Signed-off-by: Stefano Babic sba...@denx.de
---
drivers/misc/Makefile |1 +
drivers/misc/mc13892_spi_pmic.c | 134 +++
include/mc13892.h | 149 +++
3 files changed, 284 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/mc13892_spi_pmic.c
create mode 100644 include/mc13892.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index f6df60f..5847262 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -31,6 +31,7 @@ COBJS-$(CONFIG_FSL_LAW) += fsl_law.o
COBJS-$(CONFIG_NS87308) += ns87308.o
COBJS-$(CONFIG_STATUS_LED) += status_led.o
COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_MC13892_SPI_PMIC) += mc13892_spi_pmic.o
COBJS := $(COBJS-y)
SRCS := $(COBJS:.o=.c)
diff --git a/drivers/misc/mc13892_spi_pmic.c b/drivers/misc/mc13892_spi_pmic.c
new file mode 100644
index 000..f095fcc
--- /dev/null
+++ b/drivers/misc/mc13892_spi_pmic.c
@@ -0,0 +1,134 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+ *
+ * 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 config.h
+#include common.h
+#include spi.h
+#include asm/errno.h
+#include linux/types.h
+
+u32 mc13892_reg(struct spi_slave *slave, u32 reg, u32 val, u32 write)
+{
+ char tx[4];
+ char rx[4];
+ int i;
+ u32 tmp, pmic_tx, pmic_rx;
+
+ if (!slave)
+ return 0;
+
+ if (reg 63 || write 1) {
+ printf(reg num = %d is invalid. Should be less then 63\n,
+ reg);
+ return 0;
+ }
+ pmic_tx = (write 31) | (reg 25) | (val 0x00FF);
+ debug(reg=0x%x, val=0x%08x\n, reg, pmic_tx);
+
+ tmp = pmic_tx;
+ for (i = 0; i 4; i++) {
+ tx[i] = (tmp 0xFF00) 24;
+ tmp = 8;
+ }
+
+ if (spi_xfer(slave, 4 3, tx, rx,
+ SPI_XFER_BEGIN | SPI_XFER_END)) {
+ return -1;
+ }
+
+ if (write) {
+ tx[0] = ~(1 31);
+ if (spi_xfer(slave, 4 3, tx, rx,
+ SPI_XFER_BEGIN | SPI_XFER_END)) {
+ return -1;
+ }
+ }
+
+ pmic_rx = (rx[0] 24) | (rx[1] 16) | (rx[2] 8) | rx[3];
+ debug(reg=0x%x, val_read=0x%08x 0x%x 0x%x 0x%x 0x%x\n,
+ reg, pmic_rx, rx[0], rx[1], rx[2], rx[3]);
+ return pmic_rx;
+}
+
+void mc13892_show_pmic_info(struct spi_slave *slave)
+{
+ volatile u32 rev_id;
+
+ if (!slave)
+ return;
+
+ rev_id = mc13892_reg(slave, 7, 0, 0);
+ debug(PMIC ID: 0x%08x [Rev: , rev_id);
+ switch (rev_id 0x1F) {
+ case 0x1:
+ puts(1.0);
+ break;
+ case 0x9:
+ puts(1.1);
+ break;
+ case 0xA:
+ puts(1.2);
+ break;
+ case 0x10:
+ puts(2.0);
+ break;
+ case 0x11:
+ puts(2.1);
+ break;
+ case 0x18:
+ puts(3.0);
+ break;
+ case 0x19:
+ puts(3.1);
+ break;
+ case 0x1A:
+ puts(3.2);
+ break;
+ case 0x2:
+ puts(3.2A);
+ break;
+ case 0x1B:
+ puts(3.3);
+ break;
+ case 0x1D:
+ puts(3.5);
+ break;
+ default:
+ puts(unknown);
+ break;
+ }
+ puts(]\n);
+}
+
+struct spi_slave *mc13892_spi_probe(void)
+{
+ return spi_setup_slave(CONFIG_MC13892_SPI_PMIC_BUS,
+ CONFIG_MC13892_SPI_PMIC_CS,
+ CONFIG_MC13892_SPI_PMIC_CLK,
+ CONFIG_MC13892_SPI_PMIC_MODE);
+}
+
+void mc13892_spi_free(struct spi_slave *slave)
+{
+ if (slave)
+ spi_free_slave(slave);
+}
diff --git a/include/mc13892.h b/include/mc13892.h
new file mode 100644
index 000..b57e07b
--- /dev/null
+++ b/include/mc13892.h
@@ -0,0 +1,149 @@
+/*
+ * (C) Copyright 2010
+ * Stefano Babic, DENX
