Re: [PATCH v2 1/1] ARM: i.MX8M: add PCA9450 PMIC on rev-b EVK

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 11.01.23 11:17, Ahmad Fatoum wrote:
> Hi,
> 
> How do you generate your patches? Easiest is:
> 
>   git config sendemail.to barebox@lists.infradead.org
>   git send-email -3 --annotate
> 
> This will take care to number the patches correctly.
> 
> On 11.01.23 11:01, Johannes Schneider wrote:
>> Configure and setup the PMIC found on rev-b EVKs.
>> The code is algiend with how imx8mn-evk handles both
> 
> aligned*
> 
>> PMIC variants: pca9450 vs bd71837
> 
> Please note which Boards you tested this on.

Also be aware that while you now do correct PMIC setup in PBL, you still
unconditionally use a DT with only Rohm PMIC. Given that you now have
a kernel DT for evkb. Can you add it to arch/arm/dts, enable it in the
Makefile and choose one or the other DT in PBL like i.MX8MN does?

Don't forget to add fsl,imx8mm-evkb to the match list in the board.c file.

>> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
>> ---
>>  arch/arm/boards/nxp-imx8mm-evk/lowlevel.c | 51 +++++++++++++++--------
>>  1 file changed, 34 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/boards/nxp-imx8mm-evk/lowlevel.c b/arch/arm/boards/nxp-imx8mm-evk/lowlevel.c
>> index 6132df53ec..409554c2d5 100644
>> --- a/arch/arm/boards/nxp-imx8mm-evk/lowlevel.c
>> +++ b/arch/arm/boards/nxp-imx8mm-evk/lowlevel.c
>> @@ -16,6 +16,7 @@
>>  #include <mach/iomux-mx8mm.h>
>>  #include <mach/imx8m-ccm-regs.h>
>>  #include <mfd/bd71837.h>
>> +#include <mfd/pca9450.h>
>>  #include <mach/xload.h>
>>  #include <soc/imx8m/ddr.h>
>>  #include <image-metadata.h>
>> @@ -38,6 +39,25 @@ static void setup_uart(void)
>>  	putc_ll('>');
>>  }
>>  
>> +static struct pmic_config pca9450_cfg[] = {
>> +	/* BUCKxOUT_DVS0/1 control BUCK123 output */
>> +	{ PCA9450_BUCK123_DVS, 0x29 },
>> +	/*
>> +	 * increase VDD_SOC to typical value 0.95V before first
>> +	 * DRAM access, set DVS1 to 0.85v for suspend.
>> +	 * Enable DVS control through PMIC_STBY_REQ and
>> +	 * set B1_ENMODE=1 (ON by PMIC_ON_REQ=H)
>> +	 */
>> +	{ PCA9450_BUCK1OUT_DVS0, 0x1C },
>> +	/* Set DVS1 to 0.85v for suspend */
>> +	/* Enable DVS control through PMIC_STBY_REQ and set B1_ENMODE=1 (ON by PMIC_ON_REQ=H) */
>> +	{ PCA9450_BUCK1OUT_DVS1, 0x14 },
>> +	{ PCA9450_BUCK1CTRL, 0x59 },
>> +
>> +	/* set WDOG_B_CFG to cold reset */
>> +	{ PCA9450_RESET_CTRL, 0xA1 },
>> +};
>> +
>>  static struct pmic_config bd71837_cfg[] = {
>>  	/* decrease RESET key long push time from the default 10s to 10ms */
>>  	{ BD718XX_PWRONCONFIG1, 0x0 },
>> @@ -51,21 +71,6 @@ static struct pmic_config bd71837_cfg[] = {
>>  	{ BD718XX_REGLOCK, 0x11 },
>>  };
>>  
>> -static void power_init_board(void)
>> -{
> 
> Can you leave this function as-is? This will make the diff more
> readable.
> 
>> -	struct pbl_i2c *i2c;
>> -
>> -	imx8mm_setup_pad(IMX8MM_PAD_I2C1_SCL_I2C1_SCL);
>> -	imx8mm_setup_pad(IMX8MM_PAD_I2C1_SDA_I2C1_SDA);
>> -
>> -	imx8mm_early_clock_init();
>> -	imx8m_ccgr_clock_enable(IMX8M_CCM_CCGR_I2C1);
>> -
>> -	i2c = imx8m_i2c_early_init(IOMEM(MX8MQ_I2C1_BASE_ADDR));
>> -
>> -	pmic_configure(i2c, 0x4b, bd71837_cfg, ARRAY_SIZE(bd71837_cfg));
>> -}
>> -
>>  extern struct dram_timing_info imx8mm_evk_dram_timing;
>>  
>>  static void start_atf(void)
>> @@ -78,8 +83,20 @@ static void start_atf(void)
>>  	if (current_el() != 3)
>>  		return;
>>  
>> -	power_init_board();
>> -	imx8mm_ddr_init(&imx8mm_evk_dram_timing, DRAM_TYPE_LPDDR4);
>> +	imx8mm_setup_pad(IMX8MM_PAD_I2C1_SCL_I2C1_SCL);
>> +	imx8mm_setup_pad(IMX8MM_PAD_I2C1_SDA_I2C1_SDA);
>> +
>> +	imx8mm_early_clock_init();
>> +	imx8m_ccgr_clock_enable(IMX8M_CCM_CCGR_I2C1);
>> +
>> +	i2c = imx8m_i2c_early_init(IOMEM(MX8MM_I2C1_BASE_ADDR));
>> +
>> +	imx8mm_ddr_init(&imx8mm_evk_lpddr4_timing, DRAM_TYPE_LPDDR4);
>> +	if (i2c_dev_probe(i2c, 0x25, true) == 0) {
>> +		pmic_configure(i2c, 0x25, pca9450_cfg, ARRAY_SIZE(pca9450_cfg));
>> +	} else {
>> +		pmic_configure(i2c, 0x4b, bd71837_cfg, ARRAY_SIZE(bd71837_cfg));
>> +	}
> 
> Nitpick: You can drop the braces (Kernel coding style).
> 
>>  
>>  	imx8mm_load_and_start_image_via_tfa();
>>  }
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |