[Skiboot] [PATCH 2/2] platforms/ibm-fsp: Use power_ctl bit when determining slot reset method

[Gavin Shan]

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
>> reset.
>> 
>> 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.

Thanks,
Gavin