Re: [Qemu-devel] [PATCH 3/5] hw/arm/pxa2xx: Add reset method for pxa2xx_ssp

[Peter Crosthwaite]

On Fri, Jun 5, 2015 at 4:00 PM, Peter Maydell <address@hidden> wrote:
> On 5 June 2015 at 23:57, Peter Crosthwaite <address@hidden> wrote:
>> On Thu, May 28, 2015 at 5:09 AM, Peter Maydell <address@hidden> wrote:
>>> The pxa2xx_ssp device was missing a reset method; add one.
>>>
>>> Signed-off-by: Peter Maydell <address@hidden>
>>> ---
>>>  hw/arm/pxa2xx.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
>>> index fc77b44..770902f 100644
>>> --- a/hw/arm/pxa2xx.c
>>> +++ b/hw/arm/pxa2xx.c
>>> @@ -756,6 +756,22 @@ static int pxa2xx_ssp_load(QEMUFile *f, void *opaque, 
>>> int version_id)
>>>      return 0;
>>>  }
>>>
>>> +static void pxa2xx_ssp_reset(DeviceState *d)
>>> +{
>>> +    PXA2xxSSPState *s = PXA2XX_SSP(d);
>>> +
>>> +    s->enable = 0;
>>> +    s->sscr[0] = s->sscr[1] = 0;
>>> +    s->sspsp = 0;
>>> +    s->ssto = 0;
>>> +    s->ssitr = 0;
>>> +    s->sssr = 0;
>>> +    s->sstsa = 0;
>>> +    s->ssrsa = 0;
>>> +    s->ssacd = 0;
>>> +    s->rx_start = s->rx_level = 0;
>>
>> Does this need a ssp_int_update to deassert any set interrupts?
>
> No, because the thing on the other end of the irq line should also
> reset itself to an "interrupt deasserted" state. Calling qemu_set_irq
> or qemu_reset_irq in a reset function is dubious, because you don't
> know if the device on the other end has (a) already reset itself
> or (b) not yet reset itself but will do so after you; so the effect
> on the other device is indeterminate...
>

Ok so out of scope of this patches so:

Reviewed-by: Peter Crosthwaite <address@hidden>

But I have some follow up design questions, mainly in that I have
started using individual device resets that back onto the ->reset hook
to implement reset controllers. This means an IRQ source should reset
its IRQ pin to notify its sinks of the state change as those sinks
will not get a reset callback.

I know its not supported by the current reset semantic but can we
start doing this going forward?

Regards,
Peter

> -- PMM
>