Re: missing newlines in est.c printfs
On Thu, Aug 12, 2021 at 07:47:36AM +0200, Theo Buehler wrote:
> On Thu, Aug 12, 2021 at 03:01:37PM +1000, Jonathan Gray wrote:
> > On Thu, Aug 12, 2021 at 06:44:51AM +0200, Theo Buehler wrote:
> > > There doesn't seem to be a good reason for omitting the newlines here.
> > > If those are ever hit, it will look odd. Am I missing something?
> >
> > See the i386 p3_get_bus_clock() which is where this came from.
> > i386 prints the msr value and a newline.
> >
> > Wether that part is added to amd64 or removed from i386 it would be best
> > to keep them in sync as much as possible.
>
> Thanks. This syncs the print_msr code path from i386 in p3_get_bus_clock
> and adds the missing newlines in est_acpi_pss_changed() in i386.
ok jsg@
>
> Index: sys/arch/i386/i386/est.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 est.c
> --- sys/arch/i386/i386/est.c 31 Mar 2018 13:45:03 - 1.52
> +++ sys/arch/i386/i386/est.c 12 Aug 2021 05:46:12 -
> @@ -1017,14 +1017,14 @@ est_acpi_pss_changed(struct acpicpu_pss
> if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
> == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "est state");
> + "est state\n");
> return;
> }
>
> if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
> M_DEVBUF, M_NOWAIT)) == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "operating points");
> + "operating points\n");
> free(acpilist, M_DEVBUF, sizeof(*acpilist));
> return;
> }
> Index: sys/arch/amd64/amd64/est.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 est.c
> --- sys/arch/amd64/amd64/est.c11 Aug 2021 18:15:50 - 1.40
> +++ sys/arch/amd64/amd64/est.c12 Aug 2021 05:32:49 -
> @@ -187,7 +187,7 @@ p3_get_bus_clock(struct cpu_info *ci)
> default:
> printf("%s: unknown Core FSB_FREQ value %d",
> ci->ci_dev->dv_xname, bus);
> - break;
> + goto print_msr;
> }
> break;
> case 0x1c: /* Atom */
> @@ -211,13 +211,20 @@ p3_get_bus_clock(struct cpu_info *ci)
> default:
> printf("%s: unknown Atom FSB_FREQ value %d",
> ci->ci_dev->dv_xname, bus);
> - break;
> + goto print_msr;
> }
> break;
> default:
> /* no FSB on modern Intel processors */
> break;
> }
> + return;
> +print_msr:
> + /*
> + * Show the EBL_CR_POWERON MSR, so we'll at least have
> + * some extra information, such as clock ratio, etc.
> + */
> + printf(" (0x%llx)\n", rdmsr(MSR_EBL_CR_POWERON));
> }
>
> #if NACPICPU > 0
> @@ -291,14 +298,14 @@ est_acpi_pss_changed(struct acpicpu_pss
> if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
> == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "est state");
> + "est state\n");
> return;
> }
>
> if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
> M_DEVBUF, M_NOWAIT)) == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "operating points");
> + "operating points\n");
> free(acpilist, M_DEVBUF, sizeof(struct fqlist));
> return;
> }
>
>
Re: missing newlines in est.c printfs
On Thu, Aug 12, 2021 at 03:01:37PM +1000, Jonathan Gray wrote:
> On Thu, Aug 12, 2021 at 06:44:51AM +0200, Theo Buehler wrote:
> > There doesn't seem to be a good reason for omitting the newlines here.
> > If those are ever hit, it will look odd. Am I missing something?
>
> See the i386 p3_get_bus_clock() which is where this came from.
> i386 prints the msr value and a newline.
>
> Wether that part is added to amd64 or removed from i386 it would be best
> to keep them in sync as much as possible.
Thanks. This syncs the print_msr code path from i386 in p3_get_bus_clock
and adds the missing newlines in est_acpi_pss_changed() in i386.
Index: sys/arch/i386/i386/est.c
===
RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
retrieving revision 1.52
diff -u -p -r1.52 est.c
--- sys/arch/i386/i386/est.c31 Mar 2018 13:45:03 - 1.52
+++ sys/arch/i386/i386/est.c12 Aug 2021 05:46:12 -
@@ -1017,14 +1017,14 @@ est_acpi_pss_changed(struct acpicpu_pss
if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
== NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "est state");
+ "est state\n");
return;
}
if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
M_DEVBUF, M_NOWAIT)) == NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "operating points");
+ "operating points\n");
free(acpilist, M_DEVBUF, sizeof(*acpilist));
return;
}
Index: sys/arch/amd64/amd64/est.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
retrieving revision 1.40
diff -u -p -r1.40 est.c
--- sys/arch/amd64/amd64/est.c 11 Aug 2021 18:15:50 - 1.40
+++ sys/arch/amd64/amd64/est.c 12 Aug 2021 05:32:49 -
@@ -187,7 +187,7 @@ p3_get_bus_clock(struct cpu_info *ci)
default:
printf("%s: unknown Core FSB_FREQ value %d",
ci->ci_dev->dv_xname, bus);
- break;
+ goto print_msr;
}
break;
case 0x1c: /* Atom */
@@ -211,13 +211,20 @@ p3_get_bus_clock(struct cpu_info *ci)
default:
printf("%s: unknown Atom FSB_FREQ value %d",
ci->ci_dev->dv_xname, bus);
- break;
+ goto print_msr;
}
break;
default:
/* no FSB on modern Intel processors */
break;
}
+ return;
+print_msr:
+ /*
+* Show the EBL_CR_POWERON MSR, so we'll at least have
+* some extra information, such as clock ratio, etc.
+*/
+ printf(" (0x%llx)\n", rdmsr(MSR_EBL_CR_POWERON));
}
#if NACPICPU > 0
@@ -291,14 +298,14 @@ est_acpi_pss_changed(struct acpicpu_pss
if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
== NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "est state");
+ "est state\n");
return;
}
if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
M_DEVBUF, M_NOWAIT)) == NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "operating points");
+ "operating points\n");
free(acpilist, M_DEVBUF, sizeof(struct fqlist));
return;
}
Re: missing newlines in est.c printfs
On Thu, Aug 12, 2021 at 06:44:51AM +0200, Theo Buehler wrote:
> There doesn't seem to be a good reason for omitting the newlines here.
> If those are ever hit, it will look odd. Am I missing something?
See the i386 p3_get_bus_clock() which is where this came from.
i386 prints the msr value and a newline.
Wether that part is added to amd64 or removed from i386 it would be best
to keep them in sync as much as possible.
>
> Index: est.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 est.c
> --- est.c 11 Aug 2021 18:15:50 - 1.40
> +++ est.c 12 Aug 2021 04:40:14 -
> @@ -185,7 +185,7 @@ p3_get_bus_clock(struct cpu_info *ci)
> bus_clock = BUS333;
> break;
> default:
> - printf("%s: unknown Core FSB_FREQ value %d",
> + printf("%s: unknown Core FSB_FREQ value %d\n",
> ci->ci_dev->dv_xname, bus);
> break;
> }
> @@ -209,7 +209,7 @@ p3_get_bus_clock(struct cpu_info *ci)
> bus_clock = BUS200;
> break;
> default:
> - printf("%s: unknown Atom FSB_FREQ value %d",
> + printf("%s: unknown Atom FSB_FREQ value %d\n",
> ci->ci_dev->dv_xname, bus);
> break;
> }
> @@ -291,14 +291,14 @@ est_acpi_pss_changed(struct acpicpu_pss
> if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
> == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "est state");
> + "est state\n");
> return;
> }
>
> if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
> M_DEVBUF, M_NOWAIT)) == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "operating points");
> + "operating points\n");
> free(acpilist, M_DEVBUF, sizeof(struct fqlist));
> return;
> }
>
>
missing newlines in est.c printfs
There doesn't seem to be a good reason for omitting the newlines here.
If those are ever hit, it will look odd. Am I missing something?
Index: est.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
retrieving revision 1.40
diff -u -p -r1.40 est.c
--- est.c 11 Aug 2021 18:15:50 - 1.40
+++ est.c 12 Aug 2021 04:40:14 -
@@ -185,7 +185,7 @@ p3_get_bus_clock(struct cpu_info *ci)
bus_clock = BUS333;
break;
default:
- printf("%s: unknown Core FSB_FREQ value %d",
+ printf("%s: unknown Core FSB_FREQ value %d\n",
ci->ci_dev->dv_xname, bus);
break;
}
@@ -209,7 +209,7 @@ p3_get_bus_clock(struct cpu_info *ci)
bus_clock = BUS200;
break;
default:
- printf("%s: unknown Atom FSB_FREQ value %d",
+ printf("%s: unknown Atom FSB_FREQ value %d\n",
ci->ci_dev->dv_xname, bus);
break;
}
@@ -291,14 +291,14 @@ est_acpi_pss_changed(struct acpicpu_pss
if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
== NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "est state");
+ "est state\n");
return;
}
if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
M_DEVBUF, M_NOWAIT)) == NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
- "operating points");
+ "operating points\n");
free(acpilist, M_DEVBUF, sizeof(struct fqlist));
return;
}
