From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nikita Kiryanov <nikita@compulab.co.il>
Date: Sun, 01 Feb 2015 16:48:51 +0200
Subject: [U-Boot] [PATCH 02/21] common/lcd: Add command for setting
 cursor within lcd-framework
In-Reply-To: <1422624324-15431-2-git-send-email-oe5hpm@oevsv.at>
References: <1422624324-15431-1-git-send-email-oe5hpm@oevsv.at>
 <1422624324-15431-2-git-send-email-oe5hpm@oevsv.at>
Message-ID: <54CE3CD3.1030206@compulab.co.il>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Hannes,

On 01/30/2015 03:25 PM, Hannes Petermaier wrote:
> We need this function if we want to make some outputs i.e position the writing
> cursor out of u-boot scripts.

This commit message is inaccurate. Positioning the writing cursor is not in itself output.
Also, what is the use case for such a command?

>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
> ---
>   common/lcd.c |   21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>
> diff --git a/common/lcd.c b/common/lcd.c
> index cc34b8a..f418da9 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -279,12 +279,33 @@ static int do_lcd_clear(cmd_tbl_t *cmdtp, int flag, int argc,
>   	return 0;
>   }
>
> +static int do_lcd_setcursor(cmd_tbl_t *cmdtp, int flag, int argc,
> +			    char *const argv[])
> +{
> +	unsigned int col, row;
> +
> +	if (argc != 3)
> +		return CMD_RET_USAGE;
> +
> +	col = simple_strtoul(argv[1], NULL, 10);
> +	row = simple_strtoul(argv[2], NULL, 10);
> +	lcd_position_cursor(col, row);
> +
> +	return 0;
> +}
> +
>   U_BOOT_CMD(
>   	cls,	1,	1,	do_lcd_clear,
>   	"clear screen",
>   	""
>   );
>
> +U_BOOT_CMD(
> +	setcurs, 3,	1,	do_lcd_setcursor,
> +	"sets cursor for 'puts'",
> +	"    <col> <row> in character"
> +);
> +

I think it would be better if the U_BOOT_CMD macros were adjacent to the functions they
use. Also, I think this command is better suited for the lcd_console.c file.

>   /*----------------------------------------------------------------------*/
>
>   static int lcd_init(void *lcdbase)
>

-- 
Regards,
Nikita Kiryanov