Re: [PATCH] imx-image: added support for the optional 'count' parameter of the 'check data' command in the DCD.

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Giorgio,.

On Fri, Jul 03, 2020 at 06:20:17PM +0200, giorgio.nicole@arcor.de wrote:
> From: Giorgio Dal Molin <giorgio.nicole@arcor.de>
> 
> The command 'check data' can be used in the DCD table for an IMX soc
> to test the content of memory addresses, typically registers of some
> soc controller.
> This command supports an optional 'count' parameter that, when given,
> tells the ROM code how many times at most it's allowed to poll the
> memory address to meet the given logical condition. If the 'count'
> is not present in the command the ROM polls indefinitely.
> (See the IMX7D Ref. Manual at §6.6.7.2.2).
> 
> With this patch it's possible to have the following command in the DCD:
> 
>  ...
>  check 32 until_any_bit_set 0x307900c4 0x1 500
>  ...
> 
> 
> Signed-off-by: Giorgio Dal Molin <giorgio.nicole@arcor.de>
> ---
>  arch/arm/mach-imx/include/mach/imx-header.h |  2 +-
>  scripts/imx/imx-image.c                     |  4 +++-
>  scripts/imx/imx.c                           | 10 +++++++---
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/imx-header.h b/arch/arm/mach-imx/include/mach/imx-header.h
> index dc8e2eee2..57644067c 100644
> --- a/arch/arm/mach-imx/include/mach/imx-header.h
> +++ b/arch/arm/mach-imx/include/mach/imx-header.h
> @@ -106,7 +106,7 @@ struct config_data {
>  	uint32_t first_opcode;
>  	int cpu_type;
>  	int (*check)(const struct config_data *data, uint32_t cmd,
> -		     uint32_t addr, uint32_t mask);
> +			uint32_t addr, uint32_t mask, uint32_t count);
>  	int (*write_mem)(const struct config_data *data, uint32_t addr,
>  			 uint32_t val, int width, int set_bits, int clear_bits);
>  	int (*nop)(const struct config_data *data);
> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
> index ec7444a77..0cd970ea8 100644
> --- a/scripts/imx/imx-image.c
> +++ b/scripts/imx/imx-image.c
> @@ -495,7 +495,7 @@ static void write_dcd(const char *outfile)
>  }
>  
>  static int check(const struct config_data *data, uint32_t cmd, uint32_t addr,
> -		 uint32_t mask)
> +		 uint32_t mask, uint32_t count)
>  {
>  	if (data->header_version != 2) {
>  		fprintf(stderr, "DCD check command is not available or "
> @@ -512,6 +512,8 @@ static int check(const struct config_data *data, uint32_t cmd, uint32_t addr,
>  	dcdtable[curdcd++] = htobe32(cmd);
>  	dcdtable[curdcd++] = htobe32(addr);
>  	dcdtable[curdcd++] = htobe32(mask);
> +	if ( ((cmd & (0xffff<<8)) >> 8) > sizeof(uint32_t)*3 )

It's not very clear what you mean here. Can we replace this with

	if (count)

We do not have the possibility to explicitly pass a zero count anymore
then, but that doesn't seem to be very useful anyway.

> +		dcdtable[curdcd++] = htobe32(count);
>  
>  	return 0;
>  }
> diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
> index ea576c22d..879bd897e 100644
> --- a/scripts/imx/imx.c
> +++ b/scripts/imx/imx.c
> @@ -92,7 +92,7 @@ static void do_cmd_check_usage(void)
>  
>  static int do_cmd_check(struct config_data *data, int argc, char *argv[])
>  {
> -	uint32_t addr, mask, cmd;
> +	uint32_t addr, mask, cmd, count=0, len=3;

Whitespaces left and right of operators please, if in doubt pass through
./scripts/checkpatch.pl please.

Regards,
 Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox