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__)
#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));
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) */
}
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()]
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
--
2.7.4
On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> 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(-) > > Very good cleanup! Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com> I think this snippet is good, but I am just in case cc-ing Cleber and Eduardo to check if it is in accordance with any applicable guidelines of our test framework: > 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()] > 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 > -- > 2.7.4 > > >
On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote:
> On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> 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(-)
> >
> >
> Very good cleanup!
>
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> I think this snippet is good, but I am just in case cc-ing Cleber and
> Eduardo to check if it is in accordance with any applicable guidelines of
> our test framework:
Thanks for CCing us.
>
>
> > 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()]
> > 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()]
If you really want to split those lines, please indent them
properly. e.g.:
stdout_lines = [line.rstrip() for line
in result.stdout_text.splitlines()]
See PEP 8 [1] for additional examples.
Personally, I would just keep the existing
linux_ssh_mips_malta.py code as is. I don't think splitting
those lines improves readability.
[1] https://www.python.org/dev/peps/pep-0008/#indentation
--
Eduardo
On Mon, Dec 02, 2019 at 05:49:58PM -0300, Eduardo Habkost wrote: > On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote: > > On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> 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(-) > > > > > > > > Very good cleanup! > > > > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com> > > > > I think this snippet is good, but I am just in case cc-ing Cleber and > > Eduardo to check if it is in accordance with any applicable guidelines of > > our test framework: > > Thanks for CCing us. > > > > > > > > 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()] > > > 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()] > > If you really want to split those lines, please indent them > properly. e.g.: > > stdout_lines = [line.rstrip() for line > in result.stdout_text.splitlines()] > > See PEP 8 [1] for additional examples. > > Personally, I would just keep the existing > linux_ssh_mips_malta.py code as is. I don't think splitting > those lines improves readability. > > [1] https://www.python.org/dev/peps/pep-0008/#indentation > > -- > Eduardo Right. It only helps when running terminal or editor settings are limited to ~80 cols. And even in those cases, sometimes such code split across lines needs a lot of gymnastics to not degrade in readability when compared to a longer line. We're not (yet?) enforcing PEP 8, so either as Eduardo suggested, or with no changes LGTM. Cheers, - Cleber.
On Thursday, December 5, 2019, Cleber Rosa <crosa@redhat.com> wrote: > On Mon, Dec 02, 2019 at 05:49:58PM -0300, Eduardo Habkost wrote: > > On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote: > > > On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> > 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(-) > > > > > > > > > > > Very good cleanup! > > > > > > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com> > > > > > > I think this snippet is good, but I am just in case cc-ing Cleber and > > > Eduardo to check if it is in accordance with any applicable guidelines > of > > > our test framework: > > > > Thanks for CCing us. > > > > > > > > > > > > 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()] > > > > 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()] > > > > If you really want to split those lines, please indent them > > properly. e.g.: > > > > stdout_lines = [line.rstrip() for line > > in result.stdout_text.splitlines()] > > > > See PEP 8 [1] for additional examples. > > > > Personally, I would just keep the existing > > linux_ssh_mips_malta.py code as is. I don't think splitting > > those lines improves readability. > > > > [1] https://www.python.org/dev/peps/pep-0008/#indentation > > > > -- > > Eduardo > > Right. It only helps when running terminal or editor settings are > limited to ~80 cols. And even in those cases, sometimes such code > split across lines needs a lot of gymnastics to not degrade in > readability when compared to a longer line. > > We're not (yet?) enforcing PEP 8, so either as Eduardo suggested, or > with no changes LGTM. > > Eduardo, Cleber, I truly appreciate your responses and clarifications. Best regards, Aleksandar > Cheers, > - Cleber. > >
On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> 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__) > #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)); > > 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 */ > > Sorry, Filip, just an additional thing: the first several comments in the new arrangement don't begin at the same line as the rest of comments. But I can fix it while applying, no need for you to send v2 beacause of this. Thanks, Aleksandar > #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) */ > > } > > 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()] > 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 > -- > 2.7.4 > > >
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 >
On Sunday, December 1, 2019, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > 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. > > Code must be read, whatever its history or destiny is. If slated for refactoring, one reason more to be visible in less than 80 columns. This hunk improves code visibility, clearly separates two parts of a macro, and is fine to me. > #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 > > This must be fixed. Filip, you need to send v2 with the fix for build error and other improvements asked in replies to this series. > 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. > > Planing or not planing, this hunk improves code visibility, looks good to me. > } >> 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 >> >> > >
© 2016 - 2025 Red Hat, Inc.