Re: [PATCH 1/1] sunxi: Add option to prevent power-on when plug in

[Andre Przywara <andre.przywara@arm.com>]

On Fri,  7 Jan 2022 11:54:06 -0600
Chris Morgan <macroalpha82@gmail.com> wrote:

Hi Chris,

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> For sunxi boards with the AXP209, AXP221, AXP809, and AXP818 PMICs
> (plus possibly others, I only confirmed the datasheets for these),
> it is sometimes desirable to not power up whenever the device is
> plugged in. An example would be when using the NTC CHIP inside a
> PocketCHIP. This provides a configurable option to check if bit 0 of
> register 0 of the PMIC says it was powered because of a power button
> press (0) or a plug-in event (1). If the value is 1 and this option
> is selected, the device shuts down shortly after printing a message
> to console stating the reason why it's shutting down. Powering up the
> board with the power button is not affected.

Looks OK, as it's well protected, but maybe you can reword the commit
message (and the code) to not speak of preventing "power-on", but to
"prevent booting", as this is more what the patch does?
I was initially puzzled how you would achieve to no power-on in
software.

One small thing below:

> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
>  board/sunxi/board.c         | 17 ++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 2c18cf02d1..d94046dc54 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -785,6 +785,16 @@ config AXP_GPIO
>  	---help---
>  	Say Y here to enable support for the gpio pins of the axp PMIC ICs.
>  
> +config AXP_DISABLE_POWERON_VIN
> +	bool "Disable device power-on when plugged in"
> +	depends on AXP209_POWER || AXP221_POWER || AXP809_POWER || AXP818_POWER
> +	default n
> +	---help---
> +	  Say Y here to prevent the device from powering up because of a plug-in
> +	  event. When set, the device will power on briefly to determine why it
> +	  was powered on, and if it was determined because of a plug-in event
> +	  instead of a button press event it will shut back off.
> +
>  config VIDEO_SUNXI
>  	bool "Enable graphical uboot console on HDMI, LCD or VGA"
>  	depends on !MACH_SUN8I_A83T
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index fdbcd40269..9f656b56f1 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -27,6 +27,7 @@
>  #include <asm/arch/dram.h>
>  #include <asm/arch/mmc.h>
>  #include <asm/arch/prcm.h>
> +#include <asm/arch/pmic_bus.h>
>  #include <asm/arch/spl.h>
>  #include <asm/global_data.h>
>  #include <linux/delay.h>
> @@ -586,6 +587,7 @@ static void sunxi_spl_store_dram_size(phys_addr_t dram_size)
>  void sunxi_board_init(void)
>  {
>  	int power_failed = 0;
> +	__maybe_unused u8 boot_reason;

If you move this declaration ...

>  
>  #ifdef CONFIG_LED_STATUS
>  	if (IS_ENABLED(CONFIG_SPL_DRIVERS_MISC))
> @@ -601,6 +603,21 @@ void sunxi_board_init(void)
>  	defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
>  	power_failed = axp_init();
>  
> +#if IS_ENABLED(CONFIG_AXP_DISABLE_POWERON_VIN)
> +/* This behavior (bit(0) of register 0x00 and shutdown by setting bit(7) of
> + * register 0x32 appears to be common to many AXP chips, at least according to
> + * the datasheets of the AXP209, the AXP221, the AXP803, the AXP809, and the
> + * AXP818.
> + */
> +	if (!power_failed) {

... to here, you can save the __maybe_unused tag.

Cheers,
Andre

> +		pmic_bus_read(AXP_POWER_STATUS, &boot_reason);
> +		if (boot_reason & BIT(0)) {
> +			printf("Power on by charger, shutting down.\n");
> +			pmic_bus_write(0x32, BIT(7));
> +		}
> +	}
> +#endif
> +
>  #if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
>  	defined CONFIG_AXP818_POWER
>  	power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT);