[Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default

[Frederic Barrat]

Hi Lulu,

The resulting patch looks good to me. But please submit it as 2 separate 
patches: patch 1/2 for the revert and patch 2/2 for the code change. It 
makes it easier to track what is going on.

   Fred


On 09/12/2020 06:26, Lulu_Su at wistron.com wrote:
> Hi Frederic,
> 
> The attached patch has been modified according to what you mentioned.
> And use EEH to verify, the result is the same as expected.
> If you think there is no problem, I will upload this patch to Skiboot.
> Thank you very much for your methods and suggestions.
> 
> 	Lulu
> -----Original Message-----
> From: Frederic Barrat <fbarrat at linux.ibm.com>
> Sent: Wednesday, December 9, 2020 2:33 AM
> To: Lulu Su/WHQ/Wistron <Lulu_Su at wistron.com>; hegdevasant at linux.vnet.ibm.com; skiboot at lists.ozlabs.org; stewart at linux.vnet.ibm.com
> Cc: oliveroh at au1.ibm.com; skiboot-stable at lists.ozlabs.org; klausk at br.ibm.com
> Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default
> 
> Hi,
> 
> Attached is a patch for you to try. You will also need to revert the previous patch (5262cdd1b99f77bca5951fc8132f9795ef0c2b87), since we no longer need the phb4_set_dt_max_link_speed() function.
> 
>     Fred
> 
> 
> On 08/12/2020 12:50, Lulu_Su at wistron.com wrote:
>> Hi Vasant,
>>
>> Thank you for your reminder. I have a question for your opinion:
>> Finally, there is a branch used by mowgli in ibm-op-release/op-build:
>> release-mowgli, Maybe I can refer to the previous approach
>> https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3.
>> What do you think?
>>
>>
>>
>> Hi Frederic,
>>
>> Thanks for your advice,
>> I tried to make some modifications (attached files) and verified
>> through EEH, the max-link-speed of this modification will not be lost, Do you have any suggestions for this modification?
>>
>> But as you mentioned, there is an easier way to achieve this.
>> I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0?
>>
>> If there is any problem, please feel free to contact me.
>> Thank you very much.
>>
>> Best regards,
>> Lulu Su
>>
>> -----Original Message-----
>> From: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> Sent: Wednesday, December 2, 2020 3:04 PM
>> To: Frederic Barrat <fbarrat at linux.ibm.com>; Lulu Su/WHQ/Wistron
>> <Lulu_Su at wistron.com>; skiboot at lists.ozlabs.org; Stewart Smith
>> <stewart at linux.vnet.ibm.com>
>> Cc: Oliver OHalloran <oliveroh at au1.ibm.com>;
>> skiboot-stable at lists.ozlabs.org; Klaus Heinrich Kiwi
>> <klausk at br.ibm.com>
>> Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by
>> default
>>
>> On 11/9/20 10:06 PM, Frederic Barrat wrote:
>>>
>>>
>>> On 30/10/2020 03:22, Lulu Su wrote:
>>>> From: LuluTHSu<Lulu_Su at wistron.com>
>>>>
>>>> Refer to the spec. of mowgli, limit the slot to Gen3 speed.
>>>> For mowgli platform spec.
>>>>
>>>> Cc:skiboot-stable at lists.ozlabs.org
>>>> Signed-off-by: LuluTHSu<Lulu_Su at wistron.com>
>>>> ---
>>>>     hw/phb4.c                 | 21 +++++++++++++++++++++
>>>>     include/phb4.h            |  1 +
>>>>     platforms/astbmc/mowgli.c | 16 ++++++++++++++++
>>>>     3 files changed, 38 insertions(+)
>>>>
>>>> diff --git a/hw/phb4.c b/hw/phb4.c
>>>> index 17a233f..de10bb0 100644
>>>> --- a/hw/phb4.c
>>>> +++ b/hw/phb4.c
>>>> @@ -2991,6 +2991,27 @@ static unsigned int
>>>> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np)
>>>>         return max_link_speed;
>>>>     }
>>>> +/*
>>>> + * Has the same effect as the ibm,max-link-speed property.
>>>> + * i.e. sets the default link speed, while allowing NVRAM
>>>> + * overrides, etc to still take effect.
>>>> + */
>>>> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) {
>>>> +    uint64_t scr;
>>>> +    int max;
>>>> +
>>>> +    /* take into account nvram settings, etc */
>>>> +    if (pcie_max_link_speed)
>>>> +        max = pcie_max_link_speed;
>>>> +    else
>>>> +        max = new_max;
>>>> +
>>>> +    scr = phb4_read_reg(p, PHB_PCIE_SCR);
>>>> +    scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max);
>>>> +    phb4_write_reg(p, PHB_PCIE_SCR, scr); }
>>>> +
>>>
>>>
>>> This patch has already been merged, but I think there's a problem
>>> here. This should work to limit the link speed at boot, when the link
>>> is trained for the first time. However, since we're writing directly
>>> in the register, skiboot won't memorize the limit and on the next
>>> link reset/retrain (due to a EEH recovery or PCI hotplug for
>>> example), then that is lost. And mowgli_setup_phb() won't be called either on that path.
>>
>>
>> Lulu,
>>
>> Can you guys fix this issue?
>>
>> -Vasant
>>
>> ----------------------------------------------------------------------
>> ----------------------------------------------------------------------
>> ------------------- This email contains confidential or legally
>> privileged information and is for the sole use of its intended recipient.
>> Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
>> If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
>> ----------------------------------------------------------------------
>> ----------------------------------------------------------------------
>> -------------------
>>