diff mbox series

[1/4] mc146818rtc: Make PF independent of PIE

Message ID 20210613211549.18094-2-thorpej@me.com
State New
Headers show
Series Emulator fixes to enable running NetBSD/alpha | expand

Commit Message

Jason Thorpe June 13, 2021, 9:15 p.m. UTC
Make the PF flag behave like real hardware by always running the
periodic timer without regard to the setting of the PIE bit, so
that the PF will be set when the period expires even if an interrupt
will not be raised.  This behavior is documented on page 16 of the
MC146818A advance information datasheet.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hw/rtc/mc146818rtc.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Richard Henderson June 15, 2021, 4:17 a.m. UTC | #1
Cc: paolo and mst

On 6/13/21 2:15 PM, Jason Thorpe wrote:
> Make the PF flag behave like real hardware by always running the
> periodic timer without regard to the setting of the PIE bit, so
> that the PF will be set when the period expires even if an interrupt
> will not be raised.  This behavior is documented on page 16 of the
> MC146818A advance information datasheet.
> 
> Signed-off-by: Jason Thorpe <thorpej@me.com>
> ---
>   hw/rtc/mc146818rtc.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index 4fbafddb22..366b8f13de 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>   {
>       int period_code;
>   
> -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
> -        return 0;
> -     }

This looks correct, but I don't know enough about this device.


r~
Jason Thorpe June 16, 2021, 5:34 p.m. UTC | #2
> On Jun 14, 2021, at 9:17 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> Cc: paolo and mst
> 
> On 6/13/21 2:15 PM, Jason Thorpe wrote:
>> Make the PF flag behave like real hardware by always running the
>> periodic timer without regard to the setting of the PIE bit, so
>> that the PF will be set when the period expires even if an interrupt
>> will not be raised.  This behavior is documented on page 16 of the
>> MC146818A advance information datasheet.
>> Signed-off-by: Jason Thorpe <thorpej@me.com>
>> ---
>>  hw/rtc/mc146818rtc.c | 4 ----
>>  1 file changed, 4 deletions(-)
>> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>> index 4fbafddb22..366b8f13de 100644
>> --- a/hw/rtc/mc146818rtc.c
>> +++ b/hw/rtc/mc146818rtc.c
>> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>>  {
>>      int period_code;
>>  -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
>> -        return 0;
>> -     }
> 
> This looks correct, but I don't know enough about this device.

Quoting the data sheet here, for context:

<quote>
PF - The periodic interrupt flag (PF) is a read-only bit which is set to a "1" when a particular edge is detected on the selected tap of the divider chain.  The RS3 to RS0 bits establish the periodic rate.  PF is set to "1" independent of the state of the PIE bit.  PF initiates an ~IRQ signal and sets the IRQF bit when PIE is also a "1".  The PF bit is cleared by a ~RESET or a software read of Register C.
</quote>

This is why my patch always runs the timer, and does not suppress it if PF is already set; real hardware will always latch PF at regular intervals irrespective of when software reads Register C.

-- thorpej
Philippe Mathieu-Daudé June 19, 2021, 3:56 p.m. UTC | #3
Hi Jason,

On 6/16/21 7:34 PM, Jason Thorpe wrote:
> 
>> On Jun 14, 2021, at 9:17 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> Cc: paolo and mst
>>
>> On 6/13/21 2:15 PM, Jason Thorpe wrote:
>>> Make the PF flag behave like real hardware by always running the
>>> periodic timer without regard to the setting of the PIE bit, so
>>> that the PF will be set when the period expires even if an interrupt
>>> will not be raised.  This behavior is documented on page 16 of the
>>> MC146818A advance information datasheet.
>>> Signed-off-by: Jason Thorpe <thorpej@me.com>
>>> ---
>>>  hw/rtc/mc146818rtc.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>>> index 4fbafddb22..366b8f13de 100644
>>> --- a/hw/rtc/mc146818rtc.c
>>> +++ b/hw/rtc/mc146818rtc.c
>>> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>>>  {
>>>      int period_code;
>>>  -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
>>> -        return 0;
>>> -     }
>>
>> This looks correct, but I don't know enough about this device.
> 
> Quoting the data sheet here, for context:
> 
> <quote>
> PF - The periodic interrupt flag (PF) is a read-only bit which is set to a "1" when a particular edge is detected on the selected tap of the divider chain.  The RS3 to RS0 bits establish the periodic rate.  PF is set to "1" independent of the state of the PIE bit.  PF initiates an ~IRQ signal and sets the IRQF bit when PIE is also a "1".  The PF bit is cleared by a ~RESET or a software read of Register C.
> </quote>
> 
> This is why my patch always runs the timer, and does not suppress it if PF is already set; real hardware will always latch PF at regular intervals irrespective of when software reads Register C.

Do you mind reposting this single patch including the DS quote
in the description?

Regards,

Phil.
Jason Thorpe June 19, 2021, 4:25 p.m. UTC | #4
> On Jun 19, 2021, at 8:56 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> Hi Jason,
> 
> Do you mind reposting this single patch including the DS quote
> in the description?

Sure, no problem.  Will do so shortly.

-- thorpej
diff mbox series

Patch

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 4fbafddb22..366b8f13de 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -155,10 +155,6 @@  static uint32_t rtc_periodic_clock_ticks(RTCState *s)
 {
     int period_code;
 
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
-        return 0;
-     }
-
     period_code = s->cmos_data[RTC_REG_A] & 0x0f;
 
     return periodic_period_to_clock(period_code);