[Skiboot] [PATCH v6 12/16] hw/phb3: Support PHB slot

Gavin Shan gwshan at linux.vnet.ibm.com
Mon Jun 27 09:59:09 AEST 2016


On Fri, Jun 24, 2016 at 09:10:27PM +1000, Benjamin Herrenschmidt wrote:
>Hi Gavin !
>
>I was debugging a problem in simics with the IPR, which ended up being
>caused by this change in PHB3. Now I'm sure it's a bug in the SIMICS IPR
>model and I've already signaled it, however I don't understand why you
>did that change:
>
>> -       case PHB3_STATE_FRESET_DEASSERT_DELAY:
>> -               /* Switch to generic link poll state machine */
>> -               return phb3_start_link_poll(p);
>> +               pci_slot_set_state(slot,
>> +                       PHB3_SLOT_PFRESET_DEASSERT_DELAY);
>>  
>> +               return pci_slot_set_sm_timeout(slot, secs_to_tb(1));
>> +       case PHB3_SLOT_PFRESET_DEASSERT_DELAY:
>> +               pci_slot_set_state(slot, PHB3_SLOT_HRESET_START);
>> +               return slot->ops.hreset(slot);
>>         default:
>
>Basically, the old code, after a fundamental reset, would go straight
>to polling for a link.
>
>Your new code does a hot reset, then polls for the link.
>
>That isn't incorrect per-se, but it seems to me to be completely
>unnecessary... a fundamental reset is a superset of a hot reset.
>
>Also even if we can only PERST below the RC, switches are supposed
>to forward a PERST downstream as a hot reset anyway.
>

Ben, thanks for spotting it out. I agree it's unnecessary and I'll
remove this with follow-up patches. This logic (hot reset after
fundamental reset) was introduced for PCI slots on Firenze platform,
meaning a hot reset is issued after power state is changed via I2C
bus. This change was introduced to be compatible with that.

Thanks,
Gavin

>Cheers,
>Ben.
>



More information about the Skiboot mailing list