[v1] pc-bios: s390x: Cleanup part 1

[PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by Janosch Frank 4 years ago

The bios is in dire need for a cleanup as there are still a lot of
magic constants being used throughout as well as duplicated code.

In the first part of this series we consolidate constants and
functions, as well as doing some minor cleanups and fixes.

The patches are based on my protvirt branch and are available here:
https://github.com/frankjaa/qemu/pull/new/cleanup_bios

Janosch Frank (8):
  pc-bios: s390x: Consolidate timing functions into time.h
  pc-bios: s390x: Get rid of magic offsets into the lowcore
  pc-bios: s390x: Rename and use PSW_MASK_ZMODE constant
  pc-bios: s390x: Use PSW masks where possible
  pc-bios: s390x: Move panic() into header and add infinite loop
  pc-bios: s390x: Use ebcdic2ascii table
  pc-bios: s390x: Replace 0x00 with 0x0 or 0
  pc-bios: s390x: Make u32 ptr check explicit

 pc-bios/s390-ccw/bootmap.c     |  4 +---
 pc-bios/s390-ccw/cio.h         | 17 +++++++++------
 pc-bios/s390-ccw/dasd-ipl.c    | 17 +++++++--------
 pc-bios/s390-ccw/helper.h      |  2 +-
 pc-bios/s390-ccw/jump2ipl.c    | 10 ++++-----
 pc-bios/s390-ccw/main.c        | 15 +++----------
 pc-bios/s390-ccw/menu.c        |  1 +
 pc-bios/s390-ccw/netmain.c     | 23 +++-----------------
 pc-bios/s390-ccw/s390-arch.h   |  4 +++-
 pc-bios/s390-ccw/s390-ccw.h    | 25 ++++++----------------
 pc-bios/s390-ccw/start.S       |  5 +++--
 pc-bios/s390-ccw/time.h        | 39 ++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-net.c  |  1 +
 pc-bios/s390-ccw/virtio-scsi.c |  1 +
 pc-bios/s390-ccw/virtio.c      | 18 +++-------------
 15 files changed, 88 insertions(+), 94 deletions(-)
 create mode 100644 pc-bios/s390-ccw/time.h

-- 
2.25.1

Re: [PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by Janosch Frank 3 years, 11 months ago

On 3/24/20 4:08 PM, Janosch Frank wrote:
> The bios is in dire need for a cleanup as there are still a lot of
> magic constants being used throughout as well as duplicated code.
> 
> In the first part of this series we consolidate constants and
> functions, as well as doing some minor cleanups and fixes.
> 
> The patches are based on my protvirt branch and are available here:
> https://github.com/frankjaa/qemu/pull/new/cleanup_bios

Ping
It's not urgent, but I don't want it to get buried and forgotten.

> 
> Janosch Frank (8):
>   pc-bios: s390x: Consolidate timing functions into time.h
>   pc-bios: s390x: Get rid of magic offsets into the lowcore
>   pc-bios: s390x: Rename and use PSW_MASK_ZMODE constant
>   pc-bios: s390x: Use PSW masks where possible
>   pc-bios: s390x: Move panic() into header and add infinite loop
>   pc-bios: s390x: Use ebcdic2ascii table
>   pc-bios: s390x: Replace 0x00 with 0x0 or 0
>   pc-bios: s390x: Make u32 ptr check explicit
> 
>  pc-bios/s390-ccw/bootmap.c     |  4 +---
>  pc-bios/s390-ccw/cio.h         | 17 +++++++++------
>  pc-bios/s390-ccw/dasd-ipl.c    | 17 +++++++--------
>  pc-bios/s390-ccw/helper.h      |  2 +-
>  pc-bios/s390-ccw/jump2ipl.c    | 10 ++++-----
>  pc-bios/s390-ccw/main.c        | 15 +++----------
>  pc-bios/s390-ccw/menu.c        |  1 +
>  pc-bios/s390-ccw/netmain.c     | 23 +++-----------------
>  pc-bios/s390-ccw/s390-arch.h   |  4 +++-
>  pc-bios/s390-ccw/s390-ccw.h    | 25 ++++++----------------
>  pc-bios/s390-ccw/start.S       |  5 +++--
>  pc-bios/s390-ccw/time.h        | 39 ++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/virtio-net.c  |  1 +
>  pc-bios/s390-ccw/virtio-scsi.c |  1 +
>  pc-bios/s390-ccw/virtio.c      | 18 +++-------------
>  15 files changed, 88 insertions(+), 94 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/time.h
>

Re: [PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by David Hildenbrand 3 years, 11 months ago

On 29.04.20 14:11, Janosch Frank wrote:
> On 3/24/20 4:08 PM, Janosch Frank wrote:
>> The bios is in dire need for a cleanup as there are still a lot of
>> magic constants being used throughout as well as duplicated code.
>>
>> In the first part of this series we consolidate constants and
>> functions, as well as doing some minor cleanups and fixes.
>>
>> The patches are based on my protvirt branch and are available here:
>> https://github.com/frankjaa/qemu/pull/new/cleanup_bios
> 
> Ping
> It's not urgent, but I don't want it to get buried and forgotten.
> 

Let's wait for Thomas, I currently don't have any capacity to review this.


-- 
Thanks,

David / dhildenb

Re: [PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by Cornelia Huck 3 years, 11 months ago

On Thu, 30 Apr 2020 16:50:25 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 29.04.20 14:11, Janosch Frank wrote:
> > On 3/24/20 4:08 PM, Janosch Frank wrote:  
> >> The bios is in dire need for a cleanup as there are still a lot of
> >> magic constants being used throughout as well as duplicated code.
> >>
> >> In the first part of this series we consolidate constants and
> >> functions, as well as doing some minor cleanups and fixes.
> >>
> >> The patches are based on my protvirt branch and are available here:
> >> https://github.com/frankjaa/qemu/pull/new/cleanup_bios  
> > 
> > Ping
> > It's not urgent, but I don't want it to get buried and forgotten.
> >   
> 
> Let's wait for Thomas, I currently don't have any capacity to review this.
> 
> 

From the feedback, I assume there will be a v2 anyway?

Re: [PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by Janosch Frank 3 years, 11 months ago

On 5/14/20 12:59 PM, Cornelia Huck wrote:
> On Thu, 30 Apr 2020 16:50:25 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 29.04.20 14:11, Janosch Frank wrote:
>>> On 3/24/20 4:08 PM, Janosch Frank wrote:  
>>>> The bios is in dire need for a cleanup as there are still a lot of
>>>> magic constants being used throughout as well as duplicated code.
>>>>
>>>> In the first part of this series we consolidate constants and
>>>> functions, as well as doing some minor cleanups and fixes.
>>>>
>>>> The patches are based on my protvirt branch and are available here:
>>>> https://github.com/frankjaa/qemu/pull/new/cleanup_bios  
>>>
>>> Ping
>>> It's not urgent, but I don't want it to get buried and forgotten.
>>>   
>>
>> Let's wait for Thomas, I currently don't have any capacity to review this.
>>
>>
> 
> From the feedback, I assume there will be a v2 anyway?
> 

I'll send out a new version soonish(TM)

Re: [PATCH 0/8] pc-bios: s390x: Cleanup part 1

Posted by no-reply@patchew.org 4 years ago

Patchew URL: https://patchew.org/QEMU/20200324150847.10476-1-frankja@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 0/8] pc-bios: s390x: Cleanup part 1
Message-id: 20200324150847.10476-1-frankja@linux.ibm.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
631f5fb pc-bios: s390x: Make u32 ptr check explicit
31a77a2 pc-bios: s390x: Replace 0x00 with 0x0 or 0
e7c3314 pc-bios: s390x: Use ebcdic2ascii table
67bf6cc pc-bios: s390x: Move panic() into header and add infinite loop
0430d47 pc-bios: s390x: Use PSW masks where possible
5dc5f58 pc-bios: s390x: Rename and use PSW_MASK_ZMODE constant
49da8ad pc-bios: s390x: Get rid of magic offsets into the lowcore
61a8ff1 pc-bios: s390x: Consolidate timing functions into time.h

=== OUTPUT BEGIN ===
1/8 Checking commit 61a8ff1495d4 (pc-bios: s390x: Consolidate timing functions into time.h)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#102: 
new file mode 100644

total: 0 errors, 1 warnings, 167 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/8 Checking commit 49da8ad037ea (pc-bios: s390x: Get rid of magic offsets into the lowcore)
ERROR: spaces required around that ':' (ctx:VxV)
#29: FILE: pc-bios/s390-ccw/cio.h:127:
+            __u16 cssid:8;
                        ^

ERROR: code indent should never use tabs
#30: FILE: pc-bios/s390-ccw/cio.h:128:
+^I    __u16:4;$

ERROR: spaces required around that ':' (ctx:VxV)
#31: FILE: pc-bios/s390-ccw/cio.h:129:
+            __u16 m:1;
                    ^

ERROR: spaces required around that ':' (ctx:VxV)
#32: FILE: pc-bios/s390-ccw/cio.h:130:
+            __u16 ssid:2;
                       ^

ERROR: spaces required around that ':' (ctx:VxV)
#33: FILE: pc-bios/s390-ccw/cio.h:131:
+            __u16 one:1;
                      ^

total: 5 errors, 0 warnings, 37 lines checked

Patch 2/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/8 Checking commit 5dc5f5840149 (pc-bios: s390x: Rename and use PSW_MASK_ZMODE constant)
4/8 Checking commit 0430d47d5125 (pc-bios: s390x: Use PSW masks where possible)
5/8 Checking commit 67bf6cc8c901 (pc-bios: s390x: Move panic() into header and add infinite loop)
6/8 Checking commit e7c33142a550 (pc-bios: s390x: Use ebcdic2ascii table)
7/8 Checking commit 31a77a28b0f3 (pc-bios: s390x: Replace 0x00 with 0x0 or 0)
8/8 Checking commit 631f5fb35427 (pc-bios: s390x: Make u32 ptr check explicit)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200324150847.10476-1-frankja@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com