[Skiboot] [PATCH v3 3/8] capp/phb: Introduce 'struct capp' to hold capp related info in 'struct phb'
vaibhav at linux.ibm.com
Fri Jan 11 01:13:35 AEDT 2019
Thanks for reviewing this patch.
Frederic Barrat <fbarrat at linux.ibm.com> writes:
>> + capp->attached_pe = phb4_get_reserved_pe_number(&p->phb);
> It is my understanding that we just want to invalidate the stored
> attached_pe and that phb4_get_reserved_pe_number() is used, but it could
> as well be -1. It's not used anyway.
Want to use this member in future to set/remove the CAPP's IODA entries
that we populate.
> More fundamentally, why do we bother to store the attached_pe number? It
> seems it's only used when calling set_capi_mode() to go back to PCI
> mode, in which case, we don't care about the PE value.
Agreed, but sending the wrong value to the set_capi_mode() function
feels wrong and may catch some off gaurd in future.
>> + capp->chip_id = p->chip_id;
>> + /* Load capp microcode into the capp unit */
>> + rc = load_capp_ucode(p);
>> + if (rc == OPAL_SUCCESS)
>> + p->capp = capp;
>> + return rc;
> Shouldn't we free the capp memory structure if we coulnd't load the
Good catch. Will get it fixed.
Vaibhav Jain <vaibhav at linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
More information about the Skiboot