Re: [coreboot] [PATCH] ASRock E350M1: enable superio hardware monitor access

[Marc Jones]

Hi Per

On Sun, Aug 14, 2011 at 12:04 PM,   wrote:
> This patch enables access to the registers of the hardware monitor logical device
> in the superio via isa ports 0x295/0x296. Previously this was not enabled in the
> SB8xx LPC device. This is required for initialisation in init_hwm() in
> src/superio/winbond/w83627hf/superio.c and also by OS-level sensor monitoring
> such as lm-sensors to access temperature, fan monitoring and control and voltage
> registers.
>
> It might be necessary to enable this code for other superio devices using this
> register access method.
>
> The device in the Asrock E350M1 is a Nuvotech NCT5572 and shows family
> resemblance to NCT6775. But one of the 3 fan rpms shows zero. Does anyone
> have the exact 5572 datasheet?
>
> Signed-off-by: Per Hansen
>
> diff --git a/src/vendorcode/amd/cimx/sb800/SBPOR.c b/src/vendorcode/amd/cimx/sb800/SBPOR.c
> index 33d09d6..6400f76 100644
> --- a/src/vendorcode/amd/cimx/sb800/SBPOR.c
> +++ b/src/vendorcode/amd/cimx/sb800/SBPOR.c
> @@ -60,6 +60,10 @@ const static REG8MASK sbPorInitPciTable[] =
>   // LPC Device (Bus 0, Dev 20, Func 3)
>   {0x00, LPC_BUS_DEV_FUN, 0},
>   {SB_LPC_REG48, 0x00, BIT0 + BIT1 + BIT2},
> +#if CONFIG_SUPERIO_WINBOND_W83627HF == 1
> {SB_LPC_REG48 + 3, 0xFF, BIT0}, // Wide IO Port 1: enable for superio access (base addr in SB_LPC_REG66[15:0])
> {SB_LPC_REG74, 0xFF, BIT2},     // Wide IO Port 1: set width. REG74[2]=0 for 512 bytes, =1 for 16 bytes
>
> +#endif
>   {SB_LPC_REG7C, 0x00, BIT0 + BIT2},
>   {SB_LPC_REGBB, 0xFF, BIT3 + BIT4 + BIT5},
>   // A12 set 0xBB [5:3] = 111 to improve SPI timing margin.
> @@ -200,6 +204,12 @@ sbPowerOnInit (
>
> // Set Build option into SB
>   WritePCI ((LPC_BUS_DEV_FUN << 16) + SB_LPC_REG64, AccWidthUint16 | S3_SAVE, &(pConfig->BuildParameters.SioPmeBaseAddress));
> +#if CONFIG_SUPERIO_WINBOND_W83627HF == 1
> +// Set Wide IO Port 1 base address to allow access to superio HWM registers via 0x295/0x296.
> +// Enabled above in SB_LPC_REG48[24] and SB_LPC_REG74[2].
> const unsigned int p1baseaddr = 0x0290;  // if width=16 require 16 byte alignment
> WritePCI ((LPC_BUS_DEV_FUN << 16) + SB_LPC_REG66, AccWidthUint16 | S3_SAVE, &p1baseaddr);
>
> +#endif
>   RWPCI ((LPC_BUS_DEV_FUN << 16) + SB_LPC_REGA0, AccWidthUint32 | S3_SAVE, 0x001F, (pConfig->BuildParameters.SpiRomBaseAddress));
>   RWPCI ((LPC_BUS_DEV_FUN << 16) + SB_LPC_REG9C, AccWidthUint32 | S3_SAVE, 0, (pConfig->BuildParameters.GecShadowRomBase + 1));
>  // Enabled SMBUS0/SMBUS1 (ASF) Base Address
> @@ -354,4 +364,4 @@ sbPowerOnInit (
>
> // Set PMx88[5]to enable LdtStp# output to do the C3 or FidVid transation
>   RWMEM (ACPI_MMIO_BASE + PMIO_BASE + SB_PMIOA_REG88, AccWidthUint8, 0xFF, BIT5);
> -}
> \ No newline at end of file
> +}

Thanks for the patch. Should this be done in romstage per mainboard?
Maybe add new CONFIG_SB option instead of overloading the SIO config.

Kerry,

Is there already CIMx setup option for this?

Thanks,
Marc