[PATCH skeleton] Updated SCU88 register for Correct PCIe inventory

[PATCH skeleton] Updated SCU88 register for Correct PCIe inventory

[OpenBMC Patches]

Before This change:
a) SCU 88 was double assigned, first to 0x01C000FF in line:97 and then to 0x01C0007F in line:100. 
b) Which meant (in both cases, some or all of) bits 7:0 were set to 1. That is: We were reading : PWMx or VPIGx instead of GPIONx (GPIONx gives us the PCIe inventory status, where x is bit number)

After This Change:
a) Got rid of the double assignment by commenting out the second SCU88 assignment
b) Bits (7:0) of SCU 88 are set to 0 . (According to Page 111 of data sheet these have to be set to 0 for us to to read GPION0 to GPIO N7 which indicate if PCIe device is present )

Description of pins 7:0 of SCU 88:
7 RW Enable PWM7 or VPIG7 function pin  (SCU90[5:4]=0x2 select Video pin)
6 RW Enable PWM6 or VPIG6 function pin   (SCU90[5:4]=0x2 select Video pin)
5 RW Enable PWM5 or VPIG5 function pin    (SCU90[5:4]!=0 select Video pin)
4 RW Enable PWM4 or VPIG4 function pin     (SCU90[5:4]!=0 select Video pin)
3 RW Enable PWM3 or VPIG3 function pin      (SCU90[5:4]!=0 select Video pin)
2 RW Enable PWM2 or VPIG2 function pin        (SCU90[5:4]!=0 select Video pin)
1 RW Enable PWM1 or VPIG1 function pin        (SCU90[5:4]=0x3 select Video pin)
0 RW Enable PWM0 or VPIG0 function pin         (SCU90[5:4]=0x3 select Video pin)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Review on Reviewable"/>](https://reviewable.io/reviews/openbmc/skeleton/55)
<!-- Reviewable:end -->


https://github.com/openbmc/skeleton/pull/55

Adi Gangidi (1):
  Updated SCU88 register for Correct PCIe inventory

 objects/control_bmc_barreleye.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.7.1

[PATCH skeleton] Updated SCU88 register for Correct PCIe inventory

[OpenBMC Patches]

From: Adi Gangidi <adi.gangidi@rackspace.com>

Before This change:
a) SCU 88 was double assigned, first to 0x01C000FF in line:97 and then to 0x01C0007F in line:100.
b) Which meant (in both cases, some or all of) bits 7:0 were set to 1. That is: We were reading : PWMx or VPIGx instead of GPIONx (GPIONx gives us the PCIe inventory status, where x is bit number)

After This Change:
a) Got rid of the double assignment by commenting out the second SCU88 assignment
b) Bits (7:0) of SCU 88 are set to 0 . (According to Page 111 of data sheet these have to be set to 0 for us to to read GPION0 to GPIO N7 which indicate if PCIe device is present )

Description of pins 7:0 of SCU 88:
7 RW Enable PWM7 or VPIG7 function pin  (SCU90[5:4]=0x2 select Video pin)
6 RW Enable PWM6 or VPIG6 function pin   (SCU90[5:4]=0x2 select Video pin)
5 RW Enable PWM5 or VPIG5 function pin    (SCU90[5:4]!=0 select Video pin)
4 RW Enable PWM4 or VPIG4 function pin     (SCU90[5:4]!=0 select Video pin)
3 RW Enable PWM3 or VPIG3 function pin      (SCU90[5:4]!=0 select Video pin)
2 RW Enable PWM2 or VPIG2 function pin        (SCU90[5:4]!=0 select Video pin)
1 RW Enable PWM1 or VPIG1 function pin        (SCU90[5:4]=0x3 select Video pin)
0 RW Enable PWM0 or VPIG0 function pin         (SCU90[5:4]=0x3 select Video pin)
---
 objects/control_bmc_barreleye.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/objects/control_bmc_barreleye.c b/objects/control_bmc_barreleye.c
index ddfde88..0afc58d 100644
--- a/objects/control_bmc_barreleye.c
+++ b/objects/control_bmc_barreleye.c
@@ -94,10 +94,10 @@ void reg_init()
 	bmcreg = memmap(mem_fd,GPIO_BASE);
 	devmem(bmcreg+0x84,0x00fff0c0);  //Enable UART1
 	devmem(bmcreg+0x80,0xCB000000);
-	devmem(bmcreg+0x88,0x01C000FF);
+	devmem(bmcreg+0x88,0x01C00000);		//Changed Assigned value to SCU 88 for PCie inventory to work
 	devmem(bmcreg+0x8c,0xC1C000FF);
 	devmem(bmcreg+0x90,0x003FA009);
-	devmem(bmcreg+0x88,0x01C0007F);
+	// devmem(bmcreg+0x88,0x01C0007F);	// Commented out to prevent dual assignment of SCU88
 
 
 	bmcreg = memmap(mem_fd,COM_BASE);
-- 
2.7.1

Re: [PATCH skeleton] Updated SCU88 register for Correct PCIe inventory

[Norman James]

[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]


That process is no longer ran.  All scu setup is done in kernel.   We
should probably remove that whole .c file from repository.


> On Mar 9, 2016, at 9:32 AM, OpenBMC Patches <openbmc-patches@stwcx.xyz>
wrote:
>
> From: Adi Gangidi <adi.gangidi@rackspace.com>
>
> Before This change:
> a) SCU 88 was double assigned, first to 0x01C000FF in line:97 and then to
0x01C0007F in line:100.
> b) Which meant (in both cases, some or all of) bits 7:0 were set to 1.
That is: We were reading : PWMx or VPIGx instead of GPIONx (GPIONx gives us
the PCIe inventory status, where x is bit number)
>
> After This Change:
> a) Got rid of the double assignment by commenting out the second SCU88
assignment
> b) Bits (7:0) of SCU 88 are set to 0 . (According to Page 111 of data
sheet these have to be set to 0 for us to to read GPION0 to GPIO N7 which
indicate if PCIe device is present )
>
> Description of pins 7:0 of SCU 88:
> 7 RW Enable PWM7 or VPIG7 function pin  (SCU90[5:4]=0x2 select Video pin)
> 6 RW Enable PWM6 or VPIG6 function pin   (SCU90[5:4]=0x2 select Video
pin)
> 5 RW Enable PWM5 or VPIG5 function pin    (SCU90[5:4]!=0 select Video
pin)
> 4 RW Enable PWM4 or VPIG4 function pin     (SCU90[5:4]!=0 select Video
pin)
> 3 RW Enable PWM3 or VPIG3 function pin      (SCU90[5:4]!=0 select Video
pin)
> 2 RW Enable PWM2 or VPIG2 function pin        (SCU90[5:4]!=0 select Video
pin)
> 1 RW Enable PWM1 or VPIG1 function pin        (SCU90[5:4]=0x3 select
Video pin)
> 0 RW Enable PWM0 or VPIG0 function pin         (SCU90[5:4]=0x3 select
Video pin)
> ---
> objects/control_bmc_barreleye.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/objects/control_bmc_barreleye.c
b/objects/control_bmc_barreleye.c
> index ddfde88..0afc58d 100644
> --- a/objects/control_bmc_barreleye.c
> +++ b/objects/control_bmc_barreleye.c
> @@ -94,10 +94,10 @@ void reg_init()
>    bmcreg = memmap(mem_fd,GPIO_BASE);
>    devmem(bmcreg+0x84,0x00fff0c0); //Enable UART1
>    devmem(bmcreg+0x80,0xCB000000);
> -    devmem(bmcreg+0x88,0x01C000FF);
> +    devmem(bmcreg+0x88,0x01C00000);        //Changed Assigned value to
SCU 88 for PCie inventory to work
>    devmem(bmcreg+0x8c,0xC1C000FF);
>    devmem(bmcreg+0x90,0x003FA009);
> -    devmem(bmcreg+0x88,0x01C0007F);
> +    // devmem(bmcreg+0x88,0x01C0007F);    // Commented out to prevent
dual assignment of SCU88
>
>
>    bmcreg = memmap(mem_fd,COM_BASE);
> --
> 2.7.1
>
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

[-- Attachment #2: Type: text/html, Size: 2994 bytes --]

Re: [PATCH skeleton] Updated SCU88 register for Correct PCIe inventory

[Milton Miller II]

Norm James wrote:
>   That process is no longer ran. All scu setup is done in kernel. 
> We should probably remove that whole .c file from repository.
 
The kernel is writing the register to the same values in
  
do_common_setup in arch/arm/mach-aspeed/aspeed.c.  

so the concept of the patch needs to be moved there.

> 
> 
>> On Mar 9, 2016, at 9:32 AM, Adi Gangidi <adi.gangidi@rackspace.com> wrote:
>>
>> Before This change:
>> a) SCU 88 was double assigned, first to 0x01C000FF in line:97 and then to 0x01C0007F in line:100.
>> b) Which meant (in both cases, some or all of) bits 7:0 were set to 1. That is: We were reading : PWMx or VPIGx instead of GPIONx (GPIONx gives us the PCIe inventory status, where x is bit number)
>>
>> After This Change:
>> a) Got rid of the double assignment by commenting out the second SCU88 assignment
>> b) Bits (7:0) of SCU 88 are set to 0 . (According to Page 111 of data sheet these have to be set to 0 for us to to read GPION0 to GPIO N7 which indicate if PCIe device is present )
>>
>> Description of pins 7:0 of SCU 88:
>> 7 RW Enable PWM7 or VPIG7 function pin (SCU90[5:4]=0x2 select Video pin)
>> 6 RW Enable PWM6 or VPIG6 function pin (SCU90[5:4]=0x2 select Video pin)
>> 5 RW Enable PWM5 or VPIG5 function pin (SCU90[5:4]!=0 select Video pin)
>> 4 RW Enable PWM4 or VPIG4 function pin (SCU90[5:4]!=0 select Video pin)
>> 3 RW Enable PWM3 or VPIG3 function pin (SCU90[5:4]!=0 select Video pin)
>> 2 RW Enable PWM2 or VPIG2 function pin (SCU90[5:4]!=0 select Video pin)
>> 1 RW Enable PWM1 or VPIG1 function pin (SCU90[5:4]=0x3 select Video pin)
>> 0 RW Enable PWM0 or VPIG0 function pin (SCU90[5:4]=0x3 select Video pin)
>> ---

A couple of comments for the change log:  The kernel prefers the change log 
be written in the imperative, and clearly state the problem (see 
Documentation/SubmittingPatches).  Instead of describing the code before
and after describe the problem and the change.

Something like this be an improvement:

The Barreleye system uses the N0 through N5 GPIOs for inventory presence,
but the kernel is initializing the pin mux to the alternate PWM or Video in 
functions.

Initialize bits 0-7 of SCU 88 Multi-function Pin Control Register #3 to 0 
and remove the second write which had a different value only in these bit 
positions.

milton