Re: [PATCH v5 5/9] sim/erc32: Add support for LEON3 processor emulation.

[Mike Frysinger <vapier at gentoo dot org>]

On 03 Apr 2015 22:35, Jiri Gaisler wrote:
> --- /dev/null
> +++ b/sim/erc32/README.leon3
>
> +The following registers are implemeted:

"implemented"

might want to run a spell checker over the whole file

 +Cypress C601 and C602. The simulator is cycle true, i.e a simulator time is

"cycle accurate" ?

> --- a/sim/erc32/erc32.c
> +++ b/sim/erc32/erc32.c
>
> +memory_read_asi (int32 asi, uint32 addr, uint32 *data, int32 sz, int32 *ws)
> +{
> +   return memory_read (addr, data, sz, ws);

correct indentation level is 2 spaces here

> +static int
> +memory_write_asi (int32 asi, uint32 addr, uint32 *data, int32 sz, int32 *ws)
> +{
> +    return memory_write (addr, data, sz, ws);

2 spaces

> +    if (length == 4) {
> +	memory_read (addr, &w4, length, &ws);
> +	memcpy(data, &w4, length);

can you scan your patches and make sure you're using space before the (

> +                    else if ( 17 == rd ) {
> +                        if (sparclite)
> +                            sregs->asr17 = (rs1 ^ operand2);
> +                    }
> +                    else if ( 19 == rd ) {

drop the spaces with these if statements:
	else if (17 == rd) {

> +/* Vendors */

please scan all the new code to make sure the comment style is GNU

> --- a/sim/erc32/sis.h
> +++ b/sim/erc32/sis.h
> @@ -154,6 +154,29 @@ struct irqcell {
>      int32           arg;
>  };
>  
> +struct memsys {
> +    void	(*init_sim) (void);
> +    void	(*reset) (void);
> +    void	(*error_mode) (uint32 pc);
> +    void	(*sim_halt) (void);
> +    void	(*exit_sim) (void);
> +    void	(*init_stdio) (void);
> +    void	(*restore_stdio) (void);
> +    int	        (*memory_iread) (uint32 addr, uint32 *data, int32 *ws);
> +    int	        (*memory_read) (uint32 addr, uint32 *data,
> +			     int32 sz, int32 *ws);
> +    int	        (*memory_read_asi) (int32 asi, uint32 addr, uint32 *data,
> +			     int32 sz, int32 *ws);
> +    int	        (*memory_write) (uint32 addr, uint32 *data,
> +			      int32 sz, int32 *ws);
> +    int	        (*memory_write_asi) (int32 asi, uint32 addr, uint32 *data,
> +			      int32 sz, int32 *ws);
> +    int	        (*sis_memory_write) (uint32 addr,
> +				  const unsigned char *data, uint32 length);
> +    int	        (*sis_memory_read) (uint32 addr, char *data,
> +				 uint32 length);
> +    void	(*boot_init) (void);
> +};
>  
>  #define OK 0
>  #define TIME_OUT 1
> @@ -235,23 +260,7 @@ extern void	set_fsr (uint32 fsr);
>  extern void	usage (void);
>  extern void	gen_help (void);
>  
> -struct memsys {
> -    void	(*init_sim) (void);
> -    void	(*reset) (void);
> -    void	(*error_mode) (uint32 pc);
> -    void	(*sim_halt) (void);
> -    void	(*exit_sim) (void);
> -    void	(*init_stdio) (void);
> -    void	(*restore_stdio) (void);
> -    int	        (*memory_iread) (uint32 addr, uint32 *data, int32 *ws);
> -    int	        (*memory_read) (int32 asi, uint32 addr, uint32 *data,
> -			     int32 sz, int32 *ws);
> -    int	        (*memory_write) (int32 asi, uint32 addr, uint32 *data,
> -			      int32 sz, int32 *ws);
> -    int	        (*sis_memory_write) (uint32 addr,
> -				  const unsigned char *data, uint32 length);
> -    int	        (*sis_memory_read) (uint32 addr, char *data,
> -				 uint32 length);
> -};

you just added this struct in a previous patch.  if it wasn't in the right 
place, then please squash this move into that patch instead.
-mike

Attachment: signature.asc
Description: Digital signature