On Sat, Jun 27, 2015 at 02:56:33PM -0300, Paulo Alcantara wrote:
If the signal is sampled high, this indicates that the system is strapped to the "No Reboot" mode (ICH9 will disable the TCO Timer system reboot feature). The status of this strap is readable via the NO_REBOOT bit (CC: offset 0x3410:bit 5).
The NO_REBOOT bit is set when SPKR pin on ICH9 is sampled high. This bit may be set or cleared by software if the strap is sampled low but may not override the strap when it indicates "No Reboot".
This patch implements the logic where hardware has ability to set SPKR pin through a property named "pin-spkr"
I would call it "noreboot" and not pin-spkr since that's what it does in the end.
and it's sampled low by default.
I think sample high is a safer default.
Signed-off-by: Paulo Alcantara pcacjr@zytor.com
hw/acpi/tco.c | 3 ++- hw/isa/lpc_ich9.c | 38 ++++++++++++++++++++++++++++++++++++++ include/hw/i386/ich9.h | 11 +++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/tco.c b/hw/acpi/tco.c index 1794a54..c1f5739 100644 --- a/hw/acpi/tco.c +++ b/hw/acpi/tco.c @@ -64,7 +64,8 @@ static void tco_timer_expired(void *opaque) tr->tco.sts2 |= TCO_BOOT_STS; tr->timeouts_no = 0;
if (!(gcs & ICH9_CC_GCS_NO_REBOOT)) {
if ((lpc->pin_strap.spkr & ICH9_PS_SPKR_PIN_LOW) &&!(gcs & ICH9_CC_GCS_NO_REBOOT)) { watchdog_perform_action(); tco_timer_stop(tr); return;diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index b547002..49d1f30 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -575,11 +575,49 @@ static void ich9_lpc_get_sci_int(Object *obj, Visitor *v, visit_type_uint32(v, &value, name, errp); }
+static void ich9_lpc_get_spkr_pin(Object *obj, Visitor *v,
void *opaque, const char *name,Error **errp)+{
- ICH9LPCState *lpc = opaque;
- uint8_t value = lpc->pin_strap.spkr;
- visit_type_uint8(v, &value, name, errp);
+}
+static void ich9_lpc_set_spkr_pin(Object *obj, Visitor *v,
void *opaque, const char *name,Error **errp)+{
- ICH9LPCState *lpc = opaque;
- Error *local_err = NULL;
- uint8_t value;
- uint32_t *gcs;
- visit_type_uint8(v, &value, name, &local_err);
- if (local_err) {
goto out;- }
- value &= ICH9_PS_SPKR_PIN_MASK;
- if (value & ICH9_PS_SPKR_PIN_HIGH) {
gcs = (uint32_t *)&lpc->chip_config[ICH9_CC_GCS];*gcs |= ICH9_CC_GCS_NO_REBOOT;- }
- lpc->pin_strap.spkr = value;
+out:
- error_propagate(errp, local_err);
+}
static void ich9_lpc_add_properties(ICH9LPCState *lpc) { static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE; static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
lpc->pin_strap.spkr = ICH9_PS_SPKR_PIN_DEFAULT;
object_property_add(OBJECT(lpc), "pin-spkr", "uint8",
ich9_lpc_get_spkr_pin,ich9_lpc_set_spkr_pin,NULL, lpc, NULL);object_property_add(OBJECT(lpc), ACPI_PM_PROP_SCI_INT, "uint32", ich9_lpc_get_sci_int, NULL, NULL, NULL, NULL);
BTW it's easier to add simple properties in dc->props then you don't need all the error propagate code etc.
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index f5681a3..aafc43f 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -46,6 +46,11 @@ typedef struct ICH9LPCState { ICH9LPCPMRegs pm; uint32_t sci_level; /* track sci level */
- /* 2.24 Pin Straps */
- struct {
uint8_t spkr;- } pin_strap;
- /* 10.1 Chipset Configuration registers(Memory Space) which is pointed by RCBA */ uint8_t chip_config[ICH9_CC_SIZE];
@@ -72,6 +77,12 @@ Object *ich9_lpc_find(void); #define Q35_MASK(bit, ms_bit, ls_bit) \ ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
+/* ICH9: Pin Straps */ +#define ICH9_PS_SPKR_PIN_LOW 0x01 +#define ICH9_PS_SPKR_PIN_HIGH 0x02 +#define ICH9_PS_SPKR_PIN_MASK 0x03 +#define ICH9_PS_SPKR_PIN_DEFAULT ICH9_PS_SPKR_PIN_LOW
The interface seems a bit inconvenient to me. Why not make it a simple boolean property?
/* ICH9: Chipset Configuration Registers */ #define ICH9_CC_ADDR_MASK (ICH9_CC_SIZE - 1)
-- 2.1.0