Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Cédric Le Goater]

On 06/24/2016 12:55 PM, Peter Maydell wrote:
> On 24 June 2016 at 03:21, Andrew Jeffery  wrote:
>> On Thu, 2016-06-23 at 18:39 +0100, Peter Maydell wrote:
>>> On 23 June 2016 at 03:15, Andrew Jeffery  wrote:

 The magic constant configures the following options:

 * 28:27: Configure DRAM size as 256MB
 * 26:24: DDR3 SDRAM with CL = 6, CWL = 5
 * 23: Configure 24/48MHz CLKIN
 * 22: Disable GPIOE pass-through mode
 * 21: Disable GPIOD pass-through mode
 * 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
 * 19: Disable ACPI
 * 18: Configure 48MHz CLKIN
 * 17: Disable BMC 2nd boot watchdog timer
 * 16: Decode SuperIO address 0x2E
 * 15: VGA Class Code
 * 14: Enable LPC dedicated reset pin
 * 13:12: Enable SPI Master and SPI Slave to AHB Bridge
 * 11:10: Select CPU:AHB ratio = 2:1
 * 9:8: Select 384MHz H-PLL
 * 7: Configure MAC#2 for RMII/NCSI
 * 6: Configure MAC#1 for RMII/NCSI
 * 5: No VGA BIOS ROM
 * 4: Boot using 32bit SPI address mode
 * 3:2: Select 16MB VGA memory
 * 1:0: Boot from SPI flash memory
>>> Maybe we should say this in a comment in the code?
>>
>> The list describes our specific value choices in the register's
>> bitfields rather than fully documenting the bitfields and values. If
>> you prefer I could switch to the latter and make it a comment, but
>> failing that my only thought was if we tweaked the value the comment
>> maybe come out of sync. By putting our choices in the commit message
>> the description is at least accurate for what was configured at the
>> time.
> 
> I'd just like some idea of where the magic number comes
> from. At the moment the source code doesn't even have a
> reference to a data sheet that would indicate where to look.

Yes. I think the HW_STRAP1 register deserves its list of defines. 
There are some differences between the SOCs. With defines, it will 
be easier to build and read the values in the platform file. 

I can do that with a patch I will probably need to unset SPI boot.

Thanks,

C.

Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Peter Maydell]

On 24 June 2016 at 03:21, Andrew Jeffery  wrote:
> On Thu, 2016-06-23 at 18:39 +0100, Peter Maydell wrote:
>> On 23 June 2016 at 03:15, Andrew Jeffery  wrote:
>> >
>> > The magic constant configures the following options:
>> >
>> > * 28:27: Configure DRAM size as 256MB
>> > * 26:24: DDR3 SDRAM with CL = 6, CWL = 5
>> > * 23: Configure 24/48MHz CLKIN
>> > * 22: Disable GPIOE pass-through mode
>> > * 21: Disable GPIOD pass-through mode
>> > * 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
>> > * 19: Disable ACPI
>> > * 18: Configure 48MHz CLKIN
>> > * 17: Disable BMC 2nd boot watchdog timer
>> > * 16: Decode SuperIO address 0x2E
>> > * 15: VGA Class Code
>> > * 14: Enable LPC dedicated reset pin
>> > * 13:12: Enable SPI Master and SPI Slave to AHB Bridge
>> > * 11:10: Select CPU:AHB ratio = 2:1
>> > * 9:8: Select 384MHz H-PLL
>> > * 7: Configure MAC#2 for RMII/NCSI
>> > * 6: Configure MAC#1 for RMII/NCSI
>> > * 5: No VGA BIOS ROM
>> > * 4: Boot using 32bit SPI address mode
>> > * 3:2: Select 16MB VGA memory
>> > * 1:0: Boot from SPI flash memory
>> Maybe we should say this in a comment in the code?
>
> The list describes our specific value choices in the register's
> bitfields rather than fully documenting the bitfields and values. If
> you prefer I could switch to the latter and make it a comment, but
> failing that my only thought was if we tweaked the value the comment
> maybe come out of sync. By putting our choices in the commit message
> the description is at least accurate for what was configured at the
> time.

I'd just like some idea of where the magic number comes
from. At the moment the source code doesn't even have a
reference to a data sheet that would indicate where to look.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Andrew Jeffery]

On Thu, 2016-06-23 at 18:39 +0100, Peter Maydell wrote:
> On 23 June 2016 at 03:15, Andrew Jeffery  wrote:
> > 
> > The magic constant configures the following options:
> > 
> > * 28:27: Configure DRAM size as 256MB
> > * 26:24: DDR3 SDRAM with CL = 6, CWL = 5
> > * 23: Configure 24/48MHz CLKIN
> > * 22: Disable GPIOE pass-through mode
> > * 21: Disable GPIOD pass-through mode
> > * 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
> > * 19: Disable ACPI
> > * 18: Configure 48MHz CLKIN
> > * 17: Disable BMC 2nd boot watchdog timer
> > * 16: Decode SuperIO address 0x2E
> > * 15: VGA Class Code
> > * 14: Enable LPC dedicated reset pin
> > * 13:12: Enable SPI Master and SPI Slave to AHB Bridge
> > * 11:10: Select CPU:AHB ratio = 2:1
> > * 9:8: Select 384MHz H-PLL
> > * 7: Configure MAC#2 for RMII/NCSI
> > * 6: Configure MAC#1 for RMII/NCSI
> > * 5: No VGA BIOS ROM
> > * 4: Boot using 32bit SPI address mode
> > * 3:2: Select 16MB VGA memory
> > * 1:0: Boot from SPI flash memory
> Maybe we should say this in a comment in the code?

The list describes our specific value choices in the register's
bitfields rather than fully documenting the bitfields and values. If
you prefer I could switch to the latter and make it a comment, but
failing that my only thought was if we tweaked the value the comment
maybe come out of sync. By putting our choices in the commit message
the description is at least accurate for what was configured at the
time.

However I don't expect we will tweak the value...

> 
> > 
> > Signed-off-by: Andrew Jeffery 
> > ---
> >  hw/arm/palmetto-bmc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
> > index a51d960510ee..b8eed21348d8 100644
> > --- a/hw/arm/palmetto-bmc.c
> > +++ b/hw/arm/palmetto-bmc.c
> > @@ -44,6 +44,8 @@ static void palmetto_bmc_init(MachineState *machine)
> >  >ram);
> >  object_property_add_const_link(OBJECT(>soc), "ram", 
> > OBJECT(>ram),
> > _abort);
> > +object_property_set_int(OBJECT(>soc), 0x120CE416, "hw-strap1",
> > +_abort);
> >  object_property_set_bool(OBJECT(>soc), true, "realized",
> >   _abort);
> Reviewed-by: Peter Maydell 

Cheers,

Andrew


signature.asc
Description: This is a digitally signed message part

Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Peter Maydell]

On 23 June 2016 at 03:15, Andrew Jeffery  wrote:
> The magic constant configures the following options:
>
> * 28:27: Configure DRAM size as 256MB
> * 26:24: DDR3 SDRAM with CL = 6, CWL = 5
> * 23: Configure 24/48MHz CLKIN
> * 22: Disable GPIOE pass-through mode
> * 21: Disable GPIOD pass-through mode
> * 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
> * 19: Disable ACPI
> * 18: Configure 48MHz CLKIN
> * 17: Disable BMC 2nd boot watchdog timer
> * 16: Decode SuperIO address 0x2E
> * 15: VGA Class Code
> * 14: Enable LPC dedicated reset pin
> * 13:12: Enable SPI Master and SPI Slave to AHB Bridge
> * 11:10: Select CPU:AHB ratio = 2:1
> * 9:8: Select 384MHz H-PLL
> * 7: Configure MAC#2 for RMII/NCSI
> * 6: Configure MAC#1 for RMII/NCSI
> * 5: No VGA BIOS ROM
> * 4: Boot using 32bit SPI address mode
> * 3:2: Select 16MB VGA memory
> * 1:0: Boot from SPI flash memory

Maybe we should say this in a comment in the code?

> Signed-off-by: Andrew Jeffery 
> ---
>  hw/arm/palmetto-bmc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
> index a51d960510ee..b8eed21348d8 100644
> --- a/hw/arm/palmetto-bmc.c
> +++ b/hw/arm/palmetto-bmc.c
> @@ -44,6 +44,8 @@ static void palmetto_bmc_init(MachineState *machine)
>  >ram);
>  object_property_add_const_link(OBJECT(>soc), "ram", 
> OBJECT(>ram),
> _abort);
> +object_property_set_int(OBJECT(>soc), 0x120CE416, "hw-strap1",
> +_abort);
>  object_property_set_bool(OBJECT(>soc), true, "realized",
>   _abort);

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Cédric Le Goater]

On 06/23/2016 04:15 AM, Andrew Jeffery wrote:
> The magic constant configures the following options:
> 
> * 28:27: Configure DRAM size as 256MB
> * 26:24: DDR3 SDRAM with CL = 6, CWL = 5
> * 23: Configure 24/48MHz CLKIN
> * 22: Disable GPIOE pass-through mode
> * 21: Disable GPIOD pass-through mode
> * 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
> * 19: Disable ACPI
> * 18: Configure 48MHz CLKIN
> * 17: Disable BMC 2nd boot watchdog timer
> * 16: Decode SuperIO address 0x2E
> * 15: VGA Class Code
> * 14: Enable LPC dedicated reset pin
> * 13:12: Enable SPI Master and SPI Slave to AHB Bridge
> * 11:10: Select CPU:AHB ratio = 2:1
> * 9:8: Select 384MHz H-PLL
> * 7: Configure MAC#2 for RMII/NCSI
> * 6: Configure MAC#1 for RMII/NCSI
> * 5: No VGA BIOS ROM
> * 4: Boot using 32bit SPI address mode
> * 3:2: Select 16MB VGA memory
> * 1:0: Boot from SPI flash memory
> 
> Signed-off-by: Andrew Jeffery 
> ---

Reviewed-by: Cédric Le Goater 

Thanks,

C.

[Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register

[Andrew Jeffery]

The magic constant configures the following options:

* 28:27: Configure DRAM size as 256MB
* 26:24: DDR3 SDRAM with CL = 6, CWL = 5
* 23: Configure 24/48MHz CLKIN
* 22: Disable GPIOE pass-through mode
* 21: Disable GPIOD pass-through mode
* 20: Enable LPC decode of SuperIO 0x2E/0x4E addresses
* 19: Disable ACPI
* 18: Configure 48MHz CLKIN
* 17: Disable BMC 2nd boot watchdog timer
* 16: Decode SuperIO address 0x2E
* 15: VGA Class Code
* 14: Enable LPC dedicated reset pin
* 13:12: Enable SPI Master and SPI Slave to AHB Bridge
* 11:10: Select CPU:AHB ratio = 2:1
* 9:8: Select 384MHz H-PLL
* 7: Configure MAC#2 for RMII/NCSI
* 6: Configure MAC#1 for RMII/NCSI
* 5: No VGA BIOS ROM
* 4: Boot using 32bit SPI address mode
* 3:2: Select 16MB VGA memory
* 1:0: Boot from SPI flash memory

Signed-off-by: Andrew Jeffery 
---
 hw/arm/palmetto-bmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
index a51d960510ee..b8eed21348d8 100644
--- a/hw/arm/palmetto-bmc.c
+++ b/hw/arm/palmetto-bmc.c
@@ -44,6 +44,8 @@ static void palmetto_bmc_init(MachineState *machine)
 >ram);
 object_property_add_const_link(OBJECT(>soc), "ram", OBJECT(>ram),
_abort);
+object_property_set_int(OBJECT(>soc), 0x120CE416, "hw-strap1",
+_abort);
 object_property_set_bool(OBJECT(>soc), true, "realized",
  _abort);
 
-- 
2.7.4