[Skiboot] [PATCH 2/2] platforms/ibm-fsp: Use power_ctl bit when determining slot reset method
gwshan at linux.vnet.ibm.com
Mon Jul 18 16:25:35 AEST 2016
On Mon, Jul 18, 2016 at 03:49:07PM +1000, Russell Currey wrote:
>On Mon, 2016-07-18 at 15:26 +1000, Suraj Jitindar Singh wrote:
>> The power_ctl bit is used to represent if power management is available.
>> Use this bit when determining the slot reset method. If it is set then
>> we see if we can use fundamental reset, otherwise we try to use hot
>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh at gmail.com>
>> Acked-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
>Pulling out the existing power management code into its own function makes it
>hard to determine what's changed. It looks like nothing's changed except this
>setup is only performed if power management is available, right? What was the
>existing behaviour is power_ctl isn't set?
The intention is to use power_ctl. Moving the I2C stuff to a separate helper
function helps to simplify the logic and the code will be easy to be understood.
I personally prefer to have a separate function to cover the I2C bus/req here.
@power_ctl is nver used until this patch as it's not figured out correctly from
VPD from day-1.
>It's also not clear from the commit message how the reset method is affected,
>given no reset-related code is touched in this patch. The commit message and
>patch seem pretty unrelated.
I agree the commit log could be better like this:
This checks @power_ctl provided by VPD. The I2C based external power management
functionality will be populted on the PCI slot if @power_ctl is set to true.
Otherwise, we will try to use the inband PERST as fundamental reset as before.
In the meanwhile, a helper function is introduced for the logic initializing
I2C based external power management to improve the code's readability.
More information about the Skiboot