| Message ID | 20200622074235.32528-3-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc-bios: s390x: Cleanup part 1 | expand |
On 22/06/2020 09.42, Janosch Frank wrote: > Let's consolidate timing related functions into one header. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > pc-bios/s390-ccw/menu.c | 1 + > pc-bios/s390-ccw/netmain.c | 15 +++------------ > pc-bios/s390-ccw/s390-ccw.h | 8 ++++---- > pc-bios/s390-ccw/s390-time.h | 23 +++++++++++++++++++++++ > pc-bios/s390-ccw/virtio-net.c | 1 + > pc-bios/s390-ccw/virtio-scsi.c | 1 + > pc-bios/s390-ccw/virtio.c | 18 +++--------------- > 7 files changed, 36 insertions(+), 31 deletions(-) > create mode 100644 pc-bios/s390-ccw/s390-time.h > > diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c > index ce3815b201..de8260a5d6 100644 > --- a/pc-bios/s390-ccw/menu.c > +++ b/pc-bios/s390-ccw/menu.c > @@ -12,6 +12,7 @@ > #include "libc.h" > #include "s390-ccw.h" > #include "sclp.h" > +#include "s390-time.h" You could maybe drop these additional includes... > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index 21f27e7990..fae1de363f 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, > bool virtio_is_supported(SubChannelId schid); > void virtio_blk_setup_device(SubChannelId schid); > int virtio_read(ulong sector, void *load_addr); > -u64 get_clock(void); > -ulong get_second(void); > > /* bootmap.c */ > void zipl_load(void); > @@ -153,11 +151,13 @@ static inline void yield(void) > > #define MAX_SECTOR_SIZE 4096 > > +#include "s390-time.h" ... since you already include s390-time.h here in this central header file... > static inline void sleep(unsigned int seconds) > { > - ulong target = get_second() + seconds; > + ulong target = get_time_seconds() + seconds; > > - while (get_second() < target) { > + while (get_time_seconds() < target) { > yield(); > } > } ... or you could move the sleep() function into s390-time.h, too? Anyway, both ways are fine for me, so: Acked-by: Thomas Huth <thuth@redhat.com>
On 22/06/2020 12.02, Thomas Huth wrote: > On 22/06/2020 09.42, Janosch Frank wrote: >> Let's consolidate timing related functions into one header. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> pc-bios/s390-ccw/menu.c | 1 + >> pc-bios/s390-ccw/netmain.c | 15 +++------------ >> pc-bios/s390-ccw/s390-ccw.h | 8 ++++---- >> pc-bios/s390-ccw/s390-time.h | 23 +++++++++++++++++++++++ >> pc-bios/s390-ccw/virtio-net.c | 1 + >> pc-bios/s390-ccw/virtio-scsi.c | 1 + >> pc-bios/s390-ccw/virtio.c | 18 +++--------------- >> 7 files changed, 36 insertions(+), 31 deletions(-) >> create mode 100644 pc-bios/s390-ccw/s390-time.h >> >> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c >> index ce3815b201..de8260a5d6 100644 >> --- a/pc-bios/s390-ccw/menu.c >> +++ b/pc-bios/s390-ccw/menu.c >> @@ -12,6 +12,7 @@ >> #include "libc.h" >> #include "s390-ccw.h" >> #include "sclp.h" >> +#include "s390-time.h" > > You could maybe drop these additional includes... > >> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h >> index 21f27e7990..fae1de363f 100644 >> --- a/pc-bios/s390-ccw/s390-ccw.h >> +++ b/pc-bios/s390-ccw/s390-ccw.h >> @@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, >> bool virtio_is_supported(SubChannelId schid); >> void virtio_blk_setup_device(SubChannelId schid); >> int virtio_read(ulong sector, void *load_addr); >> -u64 get_clock(void); >> -ulong get_second(void); >> >> /* bootmap.c */ >> void zipl_load(void); >> @@ -153,11 +151,13 @@ static inline void yield(void) >> >> #define MAX_SECTOR_SIZE 4096 >> >> +#include "s390-time.h" > > ... since you already include s390-time.h here in this central header > file... > >> static inline void sleep(unsigned int seconds) >> { >> - ulong target = get_second() + seconds; >> + ulong target = get_time_seconds() + seconds; >> >> - while (get_second() < target) { >> + while (get_time_seconds() < target) { >> yield(); >> } >> } > > ... or you could move the sleep() function into s390-time.h, too? Never mind, I just saw that you move it in the next patch to a different header. That's fine. Thomas
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index ce3815b201..de8260a5d6 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -12,6 +12,7 @@ #include "libc.h" #include "s390-ccw.h" #include "sclp.h" +#include "s390-time.h" #define KEYCODE_NO_INP '\0' #define KEYCODE_ESCAPE '\033' diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c index 309ffa30d9..f1ee63577a 100644 --- a/pc-bios/s390-ccw/netmain.c +++ b/pc-bios/s390-ccw/netmain.c @@ -35,6 +35,7 @@ #include "s390-ccw.h" #include "cio.h" #include "virtio.h" +#include "s390-time.h" #define DEFAULT_BOOT_RETRIES 10 #define DEFAULT_TFTP_RETRIES 20 @@ -57,24 +58,14 @@ static SubChannelId net_schid = { .one = 1 }; static uint8_t mac[6]; static uint64_t dest_timer; -static uint64_t get_timer_ms(void) -{ - uint64_t clk; - - asm volatile(" stck %0 " : : "Q"(clk) : "memory"); - - /* Bit 51 is incremented each microsecond */ - return (clk >> (63 - 51)) / 1000; -} - void set_timer(int val) { - dest_timer = get_timer_ms() + val; + dest_timer = get_time_ms() + val; } int get_timer(void) { - return dest_timer - get_timer_ms(); + return dest_timer - get_time_ms(); } int get_sec_ticks(void) diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index 21f27e7990..fae1de363f 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, bool virtio_is_supported(SubChannelId schid); void virtio_blk_setup_device(SubChannelId schid); int virtio_read(ulong sector, void *load_addr); -u64 get_clock(void); -ulong get_second(void); /* bootmap.c */ void zipl_load(void); @@ -153,11 +151,13 @@ static inline void yield(void) #define MAX_SECTOR_SIZE 4096 +#include "s390-time.h" + static inline void sleep(unsigned int seconds) { - ulong target = get_second() + seconds; + ulong target = get_time_seconds() + seconds; - while (get_second() < target) { + while (get_time_seconds() < target) { yield(); } } diff --git a/pc-bios/s390-ccw/s390-time.h b/pc-bios/s390-ccw/s390-time.h new file mode 100644 index 0000000000..ed6d982371 --- /dev/null +++ b/pc-bios/s390-ccw/s390-time.h @@ -0,0 +1,23 @@ +#ifndef TIME_H +#define TIME_H + +static inline u64 get_clock(void) +{ + u64 r; + + asm volatile("stck %0" : "=Q" (r) : : "cc"); + return r; +} + +static inline u64 get_time_ms(void) +{ + /* Bit 51 is incremented each microsecond */ + return (get_clock() >> 12) / 1000; +} + +static inline u64 get_time_seconds(void) +{ + return get_time_ms() / 1000; +} + +#endif diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c index ff7f4dad25..a13f3b6fb9 100644 --- a/pc-bios/s390-ccw/virtio-net.c +++ b/pc-bios/s390-ccw/virtio-net.c @@ -19,6 +19,7 @@ #include <ethernet.h> #include "s390-ccw.h" #include "virtio.h" +#include "s390-time.h" #ifndef DEBUG_VIRTIO_NET #define DEBUG_VIRTIO_NET 0 diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c index 4fe4b9d261..7bf0be4ffa 100644 --- a/pc-bios/s390-ccw/virtio-scsi.c +++ b/pc-bios/s390-ccw/virtio-scsi.c @@ -14,6 +14,7 @@ #include "virtio.h" #include "scsi.h" #include "virtio-scsi.h" +#include "s390-time.h" static ScsiDevice default_scsi_device; static VirtioScsiCmdReq req; diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c index fb40ca9828..ab49840db8 100644 --- a/pc-bios/s390-ccw/virtio.c +++ b/pc-bios/s390-ccw/virtio.c @@ -15,6 +15,7 @@ #include "virtio-scsi.h" #include "bswap.h" #include "helper.h" +#include "s390-time.h" #define VRING_WAIT_REPLY_TIMEOUT 30 @@ -157,19 +158,6 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags) } } -u64 get_clock(void) -{ - u64 r; - - asm volatile("stck %0" : "=Q" (r) : : "cc"); - return r; -} - -ulong get_second(void) -{ - return (get_clock() >> 12) / 1000000; -} - int vr_poll(VRing *vr) { if (vr->used->idx == vr->used_idx) { @@ -194,7 +182,7 @@ int vr_poll(VRing *vr) */ int vring_wait_reply(void) { - ulong target_second = get_second() + vdev.wait_reply_timeout; + ulong target_second = get_time_seconds() + vdev.wait_reply_timeout; /* Wait for any queue to be updated by the host */ do { @@ -207,7 +195,7 @@ int vring_wait_reply(void) if (r) { return 0; } - } while (!vdev.wait_reply_timeout || (get_second() < target_second)); + } while (!vdev.wait_reply_timeout || (get_time_seconds() < target_second)); return 1; }
Let's consolidate timing related functions into one header. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- pc-bios/s390-ccw/menu.c | 1 + pc-bios/s390-ccw/netmain.c | 15 +++------------ pc-bios/s390-ccw/s390-ccw.h | 8 ++++---- pc-bios/s390-ccw/s390-time.h | 23 +++++++++++++++++++++++ pc-bios/s390-ccw/virtio-net.c | 1 + pc-bios/s390-ccw/virtio-scsi.c | 1 + pc-bios/s390-ccw/virtio.c | 18 +++--------------- 7 files changed, 36 insertions(+), 31 deletions(-) create mode 100644 pc-bios/s390-ccw/s390-time.h