[U-Boot] [PATCH] Added MC13892VK Power Management driver

[Tom <Tom.Rix@windriver.com>]

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 <sbabic@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 0000000..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 & 0x00FFFFFF);
> +	debug("reg=0x%x, val=0x%08x\n", reg, pmic_tx);
> +
> +	tmp = pmic_tx;
> +	for (i = 0; i < 4; i++) {
> +		tx[i] = (tmp & 0xFF000000) >> 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 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.

The return is a u32 value, not a pointer so the modifier 'volatile' is
not needed in the declaration of rev_id

> +	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 0000000..b57e07b
> --- /dev/null
> +++ b/include/mc13892.h
> @@ -0,0 +1,149 @@
> +/*
> + * (C) Copyright 2010
> + * Stefano Babic, DENX Software Engineering, sbabic at denx.de.
> + *
> + * (C) Copyright 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
> + */
> +
> +#ifndef __MC13892_H__
> +#define __MC13892_H__
> +
> +enum {
> +	MC13892_REG_INT_STATUS0 = 0,
> +	MC13892_REG_INT_MASK0,
> +	MC13892_REG_INT_SENSE0,
> +	MC13892_REG_INT_STATUS1,
> +	MC13892_REG_INT_MASK1,
> +	MC13892_REG_INT_SENSE1,
> +	MC13892_REG_PU_MODE_S,
> +	MC13892_REG_IDENTIFICATION,
> +	MC13892_REG_UNUSED0,
> +	MC13892_REG_ACC0,
> +	MC13892_REG_ACC1,		/*10 */
> +	MC13892_REG_UNUSED1,
> +	MC13892_REG_UNUSED2,
> +	MC13892_REG_POWER_CTL0,
> +	MC13892_REG_POWER_CTL1,
> +	MC13892_REG_POWER_CTL2,
> +	MC13892_REG_REGEN_ASSIGN,
> +	MC13892_REG_UNUSED3,
> +	MC13892_REG_MEM_A,
> +	MC13892_REG_MEM_B,
> +	MC13892_REG_RTC_TIME,		/*20 */
> +	MC13892_REG_RTC_ALARM,
> +	MC13892_REG_RTC_DAY,
> +	MC13892_REG_RTC_DAY_ALARM,
> +	MC13892_REG_SW_0,
> +	MC13892_REG_SW_1,
> +	MC13892_REG_SW_2,
> +	MC13892_REG_SW_3,
> +	MC13892_REG_SW_4,
> +	MC13892_REG_SW_5,
> +	MC13892_REG_SETTING_0,		/*30 */
> +	MC13892_REG_SETTING_1,
> +	MC13892_REG_MODE_0,
> +	MC13892_REG_MODE_1,
> +	MC13892_REG_POWER_MISC,
> +	MC13892_REG_UNUSED4,
> +	MC13892_REG_UNUSED5,
> +	MC13892_REG_UNUSED6,
> +	MC13892_REG_UNUSED7,
> +	MC13892_REG_UNUSED8,
> +	MC13892_REG_UNUSED9,		/*40 */
> +	MC13892_REG_UNUSED10,
> +	MC13892_REG_UNUSED11,
> +	MC13892_REG_ADC0,
> +	MC13892_REG_ADC1,
> +	MC13892_REG_ADC2,
> +	MC13892_REG_ADC3,
> +	MC13892_REG_ADC4,
> +	MC13892_REG_CHARGE,
> +	MC13892_REG_USB0,
> +	MC13892_REG_USB1,		/*50 */
> +	MC13892_REG_LED_CTL0,
> +	MC13892_REG_LED_CTL1,
> +	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.

> +};
> +
> +/* MC13892_REG_POWER_MISC */
> +
> +#define PWGT1SPIEN	(1 << 15)
> +#define PWGT2SPIEN	(1 << 16)
> +#define PWUP		(1 << 25)
> +
> +/* MC13892_REG_CHARGE */
> +
> +#define VCHRG0		0
> +#define VCHRG1		(1 << 1)
> +#define VCHRG2		(1 << 2)
> +#define ICHRG0		(1 << 3)
> +#define ICHRG1		(1 << 4)
> +#define ICHRG2		(1 << 5)
> +#define ICHRG3		(1 << 6)
> +#define ICHRGTR0	(1 << 7)
> +#define ICHRGTR1	(1 << 8)
> +#define ICHRGTR2	(1 << 9)
> +#define FETOVRD		(1 << 10)
> +#define FETCTRL		(1 << 11)
> +#define RVRSMODE	(1 << 13)
> +#define OVCTRL0		(1 << 15)
> +#define OVCTRL1		(1 << 16)
> +#define UCHEN		(1 << 17)
> +#define CHRGLEDEN	(1 << 18)
> +#define CHRGRAWPDEN	(1 << 19)
> +
> +/* MC13892_REG_SETTING_0/1 */
> +#define VO_1_20V	0
> +#define VO_1_30V	1
> +#define VO_1_50V	2
> +#define VO_1_80V	3
> +#define VO_1_10V	4
> +#define VO_2_00V	5
> +#define VO_2_77V	6
> +#define VO_2_40V	7
> +
> +#define VIOL		0
> +#define VDIG		4
> +#define VGEN		6
> +
> +#define mc13892_reg_write(slave, reg, value) mc13892_reg(slave, reg, value, 1)
> +#define mc13892_reg_read(slave, reg)	mc13892_reg(slave, reg, 0, 0)

These wrappers add an unneeded layer of indirection
There should be real read / write funtions

Tom