Re: [Qemu-devel] [PULL] Standard SD host controller model

[Peter Crosthwaite]

 Hi Igor,

Are you able to make these changes you have negotiated with PMM? If
you send me an interdiff that fine, or an incremental patch thats fine
- I can remake the series with your changes rolled in.

Regards,
Peter

On Sat, Jun 30, 2012 at 1:22 AM, Igor Mitsyanko <address@hidden> wrote:
> On 06/29/2012 07:01 PM, Peter Maydell wrote:
>>
>> On 29 June 2012 15:52, Igor Mitsyanko <address@hidden> wrote:
>>>
>>> On 06/29/2012 02:59 PM, Peter Maydell wrote:
>>>>
>>>> The irq code looks dubious: we seem to have a single output
>>>> irq, but the code does things like:
>>>>     qemu_set_irq(sdhci->irq, sdhci->errintsigen & sdhci->errintsts);
>>>> but in other code paths:
>>>>     qemu_set_irq(sdhci->irq, sdhci->norintsigen & sdhci->norintsts);
>>>> It seems much more likely that the hardware has various interrupt
>>>> conditions which are effectively ORed together than that there
>>>> are some cases where the interrupt line is driven by one condition
>>>> and some where it's driven by the other.
>>>
>>> Sure, full interrupt trigger condition would be
>>> if ((s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
>>>        ((s->norintsts & SDHC_NIS_INSERT) && (s->wakcon &
>>> SDHC_WKUP_ON_INS))
>>> ||
>>>        ((s->norintsts & SDHC_NIS_REMOVE) && (s->wakcon &
>>> SDHC_WKUP_ON_RMV)))
>>>
>>> but there's no point in using full check everywhere, if execution of some
>>> specific code block could result in generation of normal interrupt
>>> request
>>> only (bits in norintsts) or error interrupt request only (bits in
>>> errintsts), then its the only condition we need to check.
>>
>> But what if there was already a norintsts interrupt asserted, and this
>> code path is for the errintsts? Then if the errintsigen is such that
>> the errintsts is masked, you've just set the irq line to 0 and dropped
>> the norintsts interrupt, haven't you?
>>
>> -- PMM
>>
> Yeah, I have) it should be qemu_set_irq(sdhci->irq, sdhci->slotint) then
>