From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Filip Bozuta <Filip.Bozuta@rt-rk.com>, qemu-devel@nongnu.org
Cc: pburton@wavecomp.com, aleksandar.rikalo@rt-rk.com,
hpoussin@reactos.org, aurelien@aurel32.net,
amarkovic@wavecomp.com
Subject: Re: [PATCH 2/5] mips: malta: Renovate coding style
Date: Sun, 1 Dec 2019 21:20:00 +0100 [thread overview]
Message-ID: <7f7f98ec-6ee2-ec3d-6807-6b74af547241@redhat.com> (raw)
In-Reply-To: <1574687098-26689-3-git-send-email-Filip.Bozuta@rt-rk.com>
Hi Filip,
On 11/25/19 2:04 PM, Filip Bozuta wrote:
> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
> hw/mips/mips_malta.c
> hw/mips/gt64xxx_pci.c
> tests/acceptance/linux_ssh_mips_malta.py
>
> All these mips malta machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
> hw/mips/mips_malta.c | 139 ++++++++++++++++---------------
> tests/acceptance/linux_ssh_mips_malta.py | 6 +-
> 2 files changed, 75 insertions(+), 70 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 92e9ca5..18eafac 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
> */
>
> #if defined(DEBUG)
> -# define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
> +# define logout(fmt, ...) \
> + fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
This is deprecated code, if you look in the repository history, we
usually don't touch it, except to get rid of it. The way to get rid of
it is to replace the calls by trace events.
> #else
> # define logout(fmt, ...) ((void)0)
> #endif
> @@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion *address_space,
> MaltaFPGAState *s;
> Chardev *chr;
>
> - s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
> + s = (MaltaFPGAState *)g_new0(sizeof(MaltaFPGAState));
This change doesn't even compile... Please compile your patches before
posting them to the list.
You can find the prototype of this macro here:
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new0
>
> memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
> "malta-fpga", 0x100000);
> @@ -844,9 +845,9 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
> /* Small bootloader */
> p = (uint32_t *)base;
>
> - stl_p(p++, 0x08000000 | /* j 0x1fc00580 */
> + stl_p(p++, 0x08000000 | /* j 0x1fc00580 */
> ((run_addr + 0x580) & 0x0fffffff) >> 2);
> - stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x00000000); /* nop */
>
> /* YAMON service vector */
> stl_p(base + 0x500, run_addr + 0x0580); /* start: */
> @@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
> stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
>
> /* Load BAR registers as done by YAMON */
> - stl_p(p++, 0x3c09b400); /* lui t1, 0xb400 */
> + stl_p(p++, 0x3c09b400); /* lui t1, 0xb400 */
>
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c08df00); /* lui t0, 0xdf00 */
> + stl_p(p++, 0x3c08df00); /* lui t0, 0xdf00 */
> #else
> - stl_p(p++, 0x340800df); /* ori t0, r0, 0x00df */
> + stl_p(p++, 0x340800df); /* ori t0, r0, 0x00df */
> #endif
> - stl_p(p++, 0xad280068); /* sw t0, 0x0068(t1) */
> + stl_p(p++, 0xad280068); /* sw t0, 0x0068(t1) */
>
> - stl_p(p++, 0x3c09bbe0); /* lui t1, 0xbbe0 */
> + stl_p(p++, 0x3c09bbe0); /* lui t1, 0xbbe0 */
>
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c08c000); /* lui t0, 0xc000 */
> + stl_p(p++, 0x3c08c000); /* lui t0, 0xc000 */
> #else
> - stl_p(p++, 0x340800c0); /* ori t0, r0, 0x00c0 */
> + stl_p(p++, 0x340800c0); /* ori t0, r0, 0x00c0 */
> #endif
> - stl_p(p++, 0xad280048); /* sw t0, 0x0048(t1) */
> + stl_p(p++, 0xad280048); /* sw t0, 0x0048(t1) */
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c084000); /* lui t0, 0x4000 */
> + stl_p(p++, 0x3c084000); /* lui t0, 0x4000 */
> #else
> - stl_p(p++, 0x34080040); /* ori t0, r0, 0x0040 */
> + stl_p(p++, 0x34080040); /* ori t0, r0, 0x0040 */
> #endif
> - stl_p(p++, 0xad280050); /* sw t0, 0x0050(t1) */
> + stl_p(p++, 0xad280050); /* sw t0, 0x0050(t1) */
>
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c088000); /* lui t0, 0x8000 */
> + stl_p(p++, 0x3c088000); /* lui t0, 0x8000 */
> #else
> - stl_p(p++, 0x34080080); /* ori t0, r0, 0x0080 */
> + stl_p(p++, 0x34080080); /* ori t0, r0, 0x0080 */
> #endif
> - stl_p(p++, 0xad280058); /* sw t0, 0x0058(t1) */
> + stl_p(p++, 0xad280058); /* sw t0, 0x0058(t1) */
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c083f00); /* lui t0, 0x3f00 */
> + stl_p(p++, 0x3c083f00); /* lui t0, 0x3f00 */
> #else
> - stl_p(p++, 0x3408003f); /* ori t0, r0, 0x003f */
> + stl_p(p++, 0x3408003f); /* ori t0, r0, 0x003f */
> #endif
> - stl_p(p++, 0xad280060); /* sw t0, 0x0060(t1) */
> + stl_p(p++, 0xad280060); /* sw t0, 0x0060(t1) */
>
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c08c100); /* lui t0, 0xc100 */
> + stl_p(p++, 0x3c08c100); /* lui t0, 0xc100 */
> #else
> - stl_p(p++, 0x340800c1); /* ori t0, r0, 0x00c1 */
> + stl_p(p++, 0x340800c1); /* ori t0, r0, 0x00c1 */
> #endif
> - stl_p(p++, 0xad280080); /* sw t0, 0x0080(t1) */
> + stl_p(p++, 0xad280080); /* sw t0, 0x0080(t1) */
> #ifdef TARGET_WORDS_BIGENDIAN
> - stl_p(p++, 0x3c085e00); /* lui t0, 0x5e00 */
> + stl_p(p++, 0x3c085e00); /* lui t0, 0x5e00 */
> #else
> - stl_p(p++, 0x3408005e); /* ori t0, r0, 0x005e */
> + stl_p(p++, 0x3408005e); /* ori t0, r0, 0x005e */
> #endif
> - stl_p(p++, 0xad280088); /* sw t0, 0x0088(t1) */
> + stl_p(p++, 0xad280088); /* sw t0, 0x0088(t1) */
>
> /* Jump to kernel code */
> - stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff)); /* lui ra, high(kernel_entry) */
> - stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff)); /* ori ra, ra, low(kernel_entry) */
> - stl_p(p++, 0x03e00009); /* jalr ra */
> - stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x3c1f0000 |
> + ((kernel_entry >> 16) & 0xffff)); /* lui ra, high(kernel_entry) */
> + stl_p(p++, 0x37ff0000 |
> + (kernel_entry & 0xffff)); /* ori ra, ra, low(kernel_entry) */
> + stl_p(p++, 0x03e00009); /* jalr ra */
> + stl_p(p++, 0x00000000); /* nop */
>
> /* YAMON subroutines */
> p = (uint32_t *) (base + 0x800);
> - stl_p(p++, 0x03e00009); /* jalr ra */
> - stl_p(p++, 0x24020000); /* li v0,0 */
> + stl_p(p++, 0x03e00009); /* jalr ra */
> + stl_p(p++, 0x24020000); /* li v0,0 */
> /* 808 YAMON print */
> - stl_p(p++, 0x03e06821); /* move t5,ra */
> - stl_p(p++, 0x00805821); /* move t3,a0 */
> - stl_p(p++, 0x00a05021); /* move t2,a1 */
> - stl_p(p++, 0x91440000); /* lbu a0,0(t2) */
> - stl_p(p++, 0x254a0001); /* addiu t2,t2,1 */
> - stl_p(p++, 0x10800005); /* beqz a0,834 */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x0ff0021c); /* jal 870 */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x1000fff9); /* b 814 */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x01a00009); /* jalr t5 */
> - stl_p(p++, 0x01602021); /* move a0,t3 */
> + stl_p(p++, 0x03e06821); /* move t5,ra */
> + stl_p(p++, 0x00805821); /* move t3,a0 */
> + stl_p(p++, 0x00a05021); /* move t2,a1 */
> + stl_p(p++, 0x91440000); /* lbu a0,0(t2) */
> + stl_p(p++, 0x254a0001); /* addiu t2,t2,1 */
> + stl_p(p++, 0x10800005); /* beqz a0,834 */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x0ff0021c); /* jal 870 */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x1000fff9); /* b 814 */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x01a00009); /* jalr t5 */
> + stl_p(p++, 0x01602021); /* move a0,t3 */
> /* 0x83c YAMON print_count */
> - stl_p(p++, 0x03e06821); /* move t5,ra */
> - stl_p(p++, 0x00805821); /* move t3,a0 */
> - stl_p(p++, 0x00a05021); /* move t2,a1 */
> - stl_p(p++, 0x00c06021); /* move t4,a2 */
> - stl_p(p++, 0x91440000); /* lbu a0,0(t2) */
> - stl_p(p++, 0x0ff0021c); /* jal 870 */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x254a0001); /* addiu t2,t2,1 */
> - stl_p(p++, 0x258cffff); /* addiu t4,t4,-1 */
> - stl_p(p++, 0x1580fffa); /* bnez t4,84c */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x01a00009); /* jalr t5 */
> - stl_p(p++, 0x01602021); /* move a0,t3 */
> + stl_p(p++, 0x03e06821); /* move t5,ra */
> + stl_p(p++, 0x00805821); /* move t3,a0 */
> + stl_p(p++, 0x00a05021); /* move t2,a1 */
> + stl_p(p++, 0x00c06021); /* move t4,a2 */
> + stl_p(p++, 0x91440000); /* lbu a0,0(t2) */
> + stl_p(p++, 0x0ff0021c); /* jal 870 */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x254a0001); /* addiu t2,t2,1 */
> + stl_p(p++, 0x258cffff); /* addiu t4,t4,-1 */
> + stl_p(p++, 0x1580fffa); /* bnez t4,84c */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x01a00009); /* jalr t5 */
> + stl_p(p++, 0x01602021); /* move a0,t3 */
> /* 0x870 */
> - stl_p(p++, 0x3c08b800); /* lui t0,0xb400 */
> - stl_p(p++, 0x350803f8); /* ori t0,t0,0x3f8 */
> - stl_p(p++, 0x91090005); /* lbu t1,5(t0) */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x31290040); /* andi t1,t1,0x40 */
> - stl_p(p++, 0x1120fffc); /* beqz t1,878 <outch+0x8> */
> - stl_p(p++, 0x00000000); /* nop */
> - stl_p(p++, 0x03e00009); /* jalr ra */
> - stl_p(p++, 0xa1040000); /* sb a0,0(t0) */
> + stl_p(p++, 0x3c08b800); /* lui t0,0xb400 */
> + stl_p(p++, 0x350803f8); /* ori t0,t0,0x3f8 */
> + stl_p(p++, 0x91090005); /* lbu t1,5(t0) */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x31290040); /* andi t1,t1,0x40 */
> + stl_p(p++, 0x1120fffc); /* beqz t1,878 <outch+0x8> */
> + stl_p(p++, 0x00000000); /* nop */
> + stl_p(p++, 0x03e00009); /* jalr ra */
> + stl_p(p++, 0xa1040000); /* sb a0,0(t0) */
Are you planning to move/rename this file? Telling this now would
justify your cleanup.
>
> }
>
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e..44e1118 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -99,10 +99,12 @@ class LinuxSSH(Test):
> def ssh_command(self, command, is_root=True):
> self.ssh_logger.info(command)
> result = self.ssh_session.cmd(command)
> - stdout_lines = [line.rstrip() for line in result.stdout_text.splitlines()]
> + stdout_lines = [line.rstrip() for line
> + in result.stdout_text.splitlines()]
I think QEMU Python coding style is to align below the opening bracket.
> for line in stdout_lines:
> self.ssh_logger.info(line)
> - stderr_lines = [line.rstrip() for line in result.stderr_text.splitlines()]
> + stderr_lines = [line.rstrip() for line
> + in result.stderr_text.splitlines()]
> for line in stderr_lines:
> self.ssh_logger.warning(line)
> return stdout_lines, stderr_lines
>
next prev parent reply other threads:[~2019-12-01 20:21 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
2019-12-06 8:07 ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
2019-11-30 23:46 ` Aleksandar Markovic
2019-12-02 20:49 ` Eduardo Habkost
2019-12-05 21:27 ` Cleber Rosa
2019-12-06 8:24 ` Aleksandar Markovic
2019-12-01 0:00 ` Aleksandar Markovic
2019-12-01 20:20 ` Philippe Mathieu-Daudé [this message]
2019-12-06 8:21 ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 3/5] mips: mipssim: " Filip Bozuta
2019-12-06 8:03 ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 4/5] mips: r4000: " Filip Bozuta
2019-12-02 0:08 ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 5/5] mips: fulong 2e: " Filip Bozuta
2019-12-01 23:49 ` Aleksandar Markovic
2019-12-06 13:58 [PATCH 0/5] mips: machines: " Filip Bozuta
2019-12-06 13:58 ` [PATCH 2/5] mips: malta: " Filip Bozuta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f7f98ec-6ee2-ec3d-6807-6b74af547241@redhat.com \
--to=philmd@redhat.com \
--cc=Filip.Bozuta@rt-rk.com \
--cc=aleksandar.rikalo@rt-rk.com \
--cc=amarkovic@wavecomp.com \
--cc=aurelien@aurel32.net \
--cc=hpoussin@reactos.org \
--cc=pburton@wavecomp.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.