| Message ID | 1592213521-19390-7-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing the Channel Subsystem I/O | expand |
On 15/06/2020 11.31, Pierre Morel wrote: > The hardware gives us a good definition of the microsecond, > let's keep this information and let the routine accessing > the hardware keep all the information and return microseconds. > > Calculate delays in microseconds and take care about wrapping > around zero. > > Define values with macros and use inlines to keep the > milliseconds interface. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) In case you respin, there is a typo in the title ("caluculations") ... otherwise this can be fixed when the patch gets picked up. Reviewed-by: Thomas Huth <thuth@redhat.com>
On 2020-06-16 11:35, Thomas Huth wrote: > On 15/06/2020 11.31, Pierre Morel wrote: >> The hardware gives us a good definition of the microsecond, >> let's keep this information and let the routine accessing >> the hardware keep all the information and return microseconds. >> >> Calculate delays in microseconds and take care about wrapping >> around zero. >> >> Define values with macros and use inlines to keep the >> milliseconds interface. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- >> 1 file changed, 27 insertions(+), 2 deletions(-) > > In case you respin, there is a typo in the title ("caluculations") ... > otherwise this can be fixed when the patch gets picked up. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Yes if I respin I modifify it :) Thanks, Pierre
On Mon, 15 Jun 2020 11:31:55 +0200 Pierre Morel <pmorel@linux.ibm.com> wrote: > The hardware gives us a good definition of the microsecond, > let's keep this information and let the routine accessing > the hardware keep all the information and return microseconds. > > Calculate delays in microseconds and take care about wrapping > around zero. > > Define values with macros and use inlines to keep the > milliseconds interface. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h > index 1791380..7f1d891 100644 > --- a/lib/s390x/asm/time.h > +++ b/lib/s390x/asm/time.h > @@ -13,14 +13,39 @@ > #ifndef ASM_S390X_TIME_H > #define ASM_S390X_TIME_H > > -static inline uint64_t get_clock_ms(void) > +#define STCK_SHIFT_US (63 - 51) > +#define STCK_MAX ((1UL << 52) - 1) > + > +static inline uint64_t get_clock_us(void) > { > uint64_t clk; > > asm volatile(" stck %0 " : : "Q"(clk) : "memory"); > > /* Bit 51 is incrememented each microsecond */ That comment may now be a tad non-obvious, because the details are hidden behind the #define? Anyway, no strong opinion. > - return (clk >> (63 - 51)) / 1000; > + return clk >> STCK_SHIFT_US; > +} > + > +static inline void udelay(unsigned long us) > +{ > + unsigned long startclk = get_clock_us(); > + unsigned long c; > + > + do { > + c = get_clock_us(); > + if (c < startclk) > + c += STCK_MAX; > + } while (c < startclk + us); > +} > + > +static inline void mdelay(unsigned long ms) > +{ > + udelay(ms * 1000); > +} > + > +static inline uint64_t get_clock_ms(void) > +{ > + return get_clock_us() / 1000; > } > > #endif Acked-by: Cornelia Huck <cohuck@redhat.com>
On 2020-06-17 10:27, Cornelia Huck wrote: > On Mon, 15 Jun 2020 11:31:55 +0200 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> The hardware gives us a good definition of the microsecond, >> let's keep this information and let the routine accessing >> the hardware keep all the information and return microseconds. >> >> Calculate delays in microseconds and take care about wrapping >> around zero. >> >> Define values with macros and use inlines to keep the >> milliseconds interface. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- >> 1 file changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h >> index 1791380..7f1d891 100644 >> --- a/lib/s390x/asm/time.h >> +++ b/lib/s390x/asm/time.h >> @@ -13,14 +13,39 @@ >> #ifndef ASM_S390X_TIME_H >> #define ASM_S390X_TIME_H >> >> -static inline uint64_t get_clock_ms(void) >> +#define STCK_SHIFT_US (63 - 51) >> +#define STCK_MAX ((1UL << 52) - 1) >> + >> +static inline uint64_t get_clock_us(void) >> { >> uint64_t clk; >> >> asm volatile(" stck %0 " : : "Q"(clk) : "memory"); >> >> /* Bit 51 is incrememented each microsecond */ > > That comment may now be a tad non-obvious, because the details are > hidden behind the #define? Anyway, no strong opinion. You are right. I find the macro's name is explicit so I remove the comment. > >> - return (clk >> (63 - 51)) / 1000; >> + return clk >> STCK_SHIFT_US; >> +} >> + >> +static inline void udelay(unsigned long us) >> +{ >> + unsigned long startclk = get_clock_us(); >> + unsigned long c; >> + >> + do { >> + c = get_clock_us(); >> + if (c < startclk) >> + c += STCK_MAX; >> + } while (c < startclk + us); >> +} >> + >> +static inline void mdelay(unsigned long ms) >> +{ >> + udelay(ms * 1000); >> +} >> + >> +static inline uint64_t get_clock_ms(void) >> +{ >> + return get_clock_us() / 1000; >> } >> >> #endif > > Acked-by: Cornelia Huck <cohuck@redhat.com> > Thanks, Pierre
On 6/15/20 11:31 AM, Pierre Morel wrote: > The hardware gives us a good definition of the microsecond, > let's keep this information and let the routine accessing > the hardware keep all the information and return microseconds. > > Calculate delays in microseconds and take care about wrapping > around zero. > > Define values with macros and use inlines to keep the > milliseconds interface. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Small nit below. Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h > index 1791380..7f1d891 100644 > --- a/lib/s390x/asm/time.h > +++ b/lib/s390x/asm/time.h > @@ -13,14 +13,39 @@ > #ifndef ASM_S390X_TIME_H > #define ASM_S390X_TIME_H > > -static inline uint64_t get_clock_ms(void) > +#define STCK_SHIFT_US (63 - 51) > +#define STCK_MAX ((1UL << 52) - 1) > + > +static inline uint64_t get_clock_us(void) > { > uint64_t clk; > > asm volatile(" stck %0 " : : "Q"(clk) : "memory"); > > /* Bit 51 is incrememented each microsecond */ > - return (clk >> (63 - 51)) / 1000; > + return clk >> STCK_SHIFT_US; > +} > + > +static inline void udelay(unsigned long us) > +{ > + unsigned long startclk = get_clock_us(); > + unsigned long c; > + > + do { > + c = get_clock_us(); > + if (c < startclk) > + c += STCK_MAX; > + } while (c < startclk + us); > +} > + > +static inline void mdelay(unsigned long ms) > +{ > + udelay(ms * 1000); > +} > + > +static inline uint64_t get_clock_ms(void) > +{ > + return get_clock_us() / 1000; > } Why don't you put that below to the get_clock_us()? > > #endif >
On 2020-06-22 11:09, Janosch Frank wrote: > On 6/15/20 11:31 AM, Pierre Morel wrote: >> The hardware gives us a good definition of the microsecond, >> let's keep this information and let the routine accessing >> the hardware keep all the information and return microseconds. >> >> Calculate delays in microseconds and take care about wrapping >> around zero. >> >> Define values with macros and use inlines to keep the >> milliseconds interface. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > Small nit below. seen, I move it up. > > > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Thanks, Pierre > >> --- >> lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- >> 1 file changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h >> index 1791380..7f1d891 100644 >> --- a/lib/s390x/asm/time.h >> +++ b/lib/s390x/asm/time.h >> @@ -13,14 +13,39 @@ >> #ifndef ASM_S390X_TIME_H >> #define ASM_S390X_TIME_H >> >> -static inline uint64_t get_clock_ms(void) >> +#define STCK_SHIFT_US (63 - 51) >> +#define STCK_MAX ((1UL << 52) - 1) >> + >> +static inline uint64_t get_clock_us(void) >> { >> uint64_t clk; >> >> asm volatile(" stck %0 " : : "Q"(clk) : "memory"); >> >> /* Bit 51 is incrememented each microsecond */ >> - return (clk >> (63 - 51)) / 1000; >> + return clk >> STCK_SHIFT_US; >> +} >> + >> +static inline void udelay(unsigned long us) >> +{ >> + unsigned long startclk = get_clock_us(); >> + unsigned long c; >> + >> + do { >> + c = get_clock_us(); >> + if (c < startclk) >> + c += STCK_MAX; >> + } while (c < startclk + us); >> +} >> + >> +static inline void mdelay(unsigned long ms) >> +{ >> + udelay(ms * 1000); >> +} >> + >> +static inline uint64_t get_clock_ms(void) >> +{ >> + return get_clock_us() / 1000; >> } > > Why don't you put that below to the get_clock_us()? right, better.
diff --git a/lib/s390x/asm/time.h b/lib/s390x/asm/time.h index 1791380..7f1d891 100644 --- a/lib/s390x/asm/time.h +++ b/lib/s390x/asm/time.h @@ -13,14 +13,39 @@ #ifndef ASM_S390X_TIME_H #define ASM_S390X_TIME_H -static inline uint64_t get_clock_ms(void) +#define STCK_SHIFT_US (63 - 51) +#define STCK_MAX ((1UL << 52) - 1) + +static inline uint64_t get_clock_us(void) { uint64_t clk; asm volatile(" stck %0 " : : "Q"(clk) : "memory"); /* Bit 51 is incrememented each microsecond */ - return (clk >> (63 - 51)) / 1000; + return clk >> STCK_SHIFT_US; +} + +static inline void udelay(unsigned long us) +{ + unsigned long startclk = get_clock_us(); + unsigned long c; + + do { + c = get_clock_us(); + if (c < startclk) + c += STCK_MAX; + } while (c < startclk + us); +} + +static inline void mdelay(unsigned long ms) +{ + udelay(ms * 1000); +} + +static inline uint64_t get_clock_ms(void) +{ + return get_clock_us() / 1000; } #endif
The hardware gives us a good definition of the microsecond, let's keep this information and let the routine accessing the hardware keep all the information and return microseconds. Calculate delays in microseconds and take care about wrapping around zero. Define values with macros and use inlines to keep the milliseconds interface. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- lib/s390x/asm/time.h | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)