platforms/witherspoon: Make PCIe shared slot error message more informative

Message ID 20181122062014.24896-1-andrew.donnellan@au1.ibm.com
State Accepted
Headers show
Series
  • platforms/witherspoon: Make PCIe shared slot error message more informative
Related show

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied

Commit Message

Andrew Donnellan Nov. 22, 2018, 6:20 a.m.
If we're missing chips for some reason, we print a warning when configuring
the PCIe shared slot.

The warning doesn't really make it clear what "shared slot" is, and if it's
printed, it'll come right after a bunch of messages about NPU setup, so
let's clarify the message to explicitly mention PCI.

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 platforms/astbmc/witherspoon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Oliver Nov. 22, 2018, 6:41 a.m. | #1
On Thu, Nov 22, 2018 at 5:20 PM Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
>
> If we're missing chips for some reason, we print a warning when configuring
> the PCIe shared slot.
>
> The warning doesn't really make it clear what "shared slot" is, and if it's
> printed, it'll come right after a bunch of messages about NPU setup, so
> let's clarify the message to explicitly mention PCI.
>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
>  platforms/astbmc/witherspoon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
> index fe138991696f..0fafc87eb4c6 100644
> --- a/platforms/astbmc/witherspoon.c
> +++ b/platforms/astbmc/witherspoon.c
> @@ -184,7 +184,7 @@ static void witherspoon_shared_slot_fixup(void)
>         chip1 = next_chip(chip0);
>         if (!chip1 || next_chip(chip1)) {
>                 prlog(PR_WARNING,
> -                       "Unexpected number of chips, skipping shared slot detection\n");
> +                       "PLAT: Unexpected number of chips, skipping PCIe shared slot detection\n");

WELL PERSONALLY I THINK THAT THIS WOULD MAKE MORE SENSE IF YOU SAID
"SKIPPING DETECTION OF SHARED PCIE SLOT" AND POSSIBLY SAID SOMETHING
ABOUT MISSING A SECOND CHIP RATHER THAN "UNEXPECTED NUMBER OF CHIPS" I
MEAN REALLY, THE SYSTEM HAS TWO SOCKETS WHAT ELSE COULD IT POSSIBLY
BE????!

please fix, thank you

Oliver
Director, Paint-it-red campaign
Foundation for Bike Shed Aesthetics
Myers-Brigg Type Indicator: LOL with characteristics of WUT
Andrew Donnellan Nov. 22, 2018, 11:46 p.m. | #2
On 22/11/18 5:41 pm, Oliver wrote:
> WELL PERSONALLY I THINK THAT THIS WOULD MAKE MORE SENSE IF YOU SAID
> "SKIPPING DETECTION OF SHARED PCIE SLOT" AND POSSIBLY SAID SOMETHING
> ABOUT MISSING A SECOND CHIP RATHER THAN "UNEXPECTED NUMBER OF CHIPS" I
> MEAN REALLY, THE SYSTEM HAS TWO SOCKETS WHAT ELSE COULD IT POSSIBLY
> BE????!
> 
> please fix, thank you
> 
> Oliver
> Director, Paint-it-red campaign
> Foundation for Bike Shed Aesthetics
> Myers-Brigg Type Indicator: LOL with characteristics of WUT

I think you mistyped

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Oliver Nov. 23, 2018, 12:34 a.m. | #3
On Fri, Nov 23, 2018 at 10:47 AM Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
>
> On 22/11/18 5:41 pm, Oliver wrote:
> > WELL PERSONALLY I THINK THAT THIS WOULD MAKE MORE SENSE IF YOU SAID
> > "SKIPPING DETECTION OF SHARED PCIE SLOT" AND POSSIBLY SAID SOMETHING
> > ABOUT MISSING A SECOND CHIP RATHER THAN "UNEXPECTED NUMBER OF CHIPS" I
> > MEAN REALLY, THE SYSTEM HAS TWO SOCKETS WHAT ELSE COULD IT POSSIBLY
> > BE????!
> >
> > please fix, thank you
> >
> > Oliver
> > Director, Paint-it-red campaign
> > Foundation for Bike Shed Aesthetics
> > Myers-Brigg Type Indicator: LOL with characteristics of WUT
>
> I think you mistyped
>
> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

I THINK NOT

>
> --
> Andrew Donnellan              OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com  IBM Australia Limited
>
Stewart Smith Feb. 19, 2019, 3:19 a.m. | #4
Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
> If we're missing chips for some reason, we print a warning when configuring
> the PCIe shared slot.
>
> The warning doesn't really make it clear what "shared slot" is, and if it's
> printed, it'll come right after a bunch of messages about NPU setup, so
> let's clarify the message to explicitly mention PCI.
>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
>  platforms/astbmc/witherspoon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
> index fe138991696f..0fafc87eb4c6 100644
> --- a/platforms/astbmc/witherspoon.c
> +++ b/platforms/astbmc/witherspoon.c
> @@ -184,7 +184,7 @@ static void witherspoon_shared_slot_fixup(void)
>  	chip1 = next_chip(chip0);
>  	if (!chip1 || next_chip(chip1)) {
>  		prlog(PR_WARNING,
> -			"Unexpected number of chips, skipping shared slot detection\n");
> +			"PLAT: Unexpected number of chips, skipping PCIe shared slot detection\n");
>  		return;
>  	}

After much bikeshedding and procrastination on my part... I'm not sure
what would really be an improvement here.. but I AM GOING TO BIKESHED
THIS A BIT MORE.

going with this:
      "PLAT: Can't find second chip, "
      "skipping PCIe shared slot detection\n");
Stewart Smith Feb. 19, 2019, 4:47 a.m. | #5
Stewart Smith <stewart@linux.ibm.com> writes:
> Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
>> If we're missing chips for some reason, we print a warning when configuring
>> the PCIe shared slot.
>>
>> The warning doesn't really make it clear what "shared slot" is, and if it's
>> printed, it'll come right after a bunch of messages about NPU setup, so
>> let's clarify the message to explicitly mention PCI.
>>
>> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> ---
>>  platforms/astbmc/witherspoon.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
>> index fe138991696f..0fafc87eb4c6 100644
>> --- a/platforms/astbmc/witherspoon.c
>> +++ b/platforms/astbmc/witherspoon.c
>> @@ -184,7 +184,7 @@ static void witherspoon_shared_slot_fixup(void)
>>  	chip1 = next_chip(chip0);
>>  	if (!chip1 || next_chip(chip1)) {
>>  		prlog(PR_WARNING,
>> -			"Unexpected number of chips, skipping shared slot detection\n");
>> +			"PLAT: Unexpected number of chips, skipping PCIe shared slot detection\n");
>>  		return;
>>  	}
>
> After much bikeshedding and procrastination on my part... I'm not sure
> what would really be an improvement here.. but I AM GOING TO BIKESHED
> THIS A BIT MORE.
>
> going with this:
>       "PLAT: Can't find second chip, "
>       "skipping PCIe shared slot detection\n");

and with the added bike shed, merged to master as of 682fa68e577c0ff40f52fdec45df36dd4599b9db

Patch

diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
index fe138991696f..0fafc87eb4c6 100644
--- a/platforms/astbmc/witherspoon.c
+++ b/platforms/astbmc/witherspoon.c
@@ -184,7 +184,7 @@  static void witherspoon_shared_slot_fixup(void)
 	chip1 = next_chip(chip0);
 	if (!chip1 || next_chip(chip1)) {
 		prlog(PR_WARNING,
-			"Unexpected number of chips, skipping shared slot detection\n");
+			"PLAT: Unexpected number of chips, skipping PCIe shared slot detection\n");
 		return;
 	}