linux-kernel - Re: [PATCH v1 1/2] dt-bindings: PCI: dwc: Add snps,skip-wait-link-up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28c72c1a-df7d-db05-f32f-23b170b96db5@linaro.org>
Date:   Sat, 25 Feb 2023 11:00:48 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Sajid Dalvi <sdalvi@...gle.com>, Jingoo Han <jingoohan1@...il.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Rob Herring <robh@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     kernel-team@...roid.com, linux-pci@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: PCI: dwc: Add snps,skip-wait-link-up

On 24/02/2023 23:09, Sajid Dalvi wrote:
> On Fri, Feb 24, 2023 at 3:29 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org> wrote:
>>
>> On 24/02/2023 22:27, Sajid Dalvi wrote:
>>> On Fri, Feb 24, 2023 at 2:40 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@...aro.org> wrote:
>>>>
>>>> On 24/02/2023 20:57, Sajid Dalvi wrote:
>>>>> When the Root Complex is probed, the default behavior is to spin in a loop
>>>>> waiting for the link to come up. In some systems the link is not brought up
>>>>> during probe, but later in the context of an end-point turning on.
>>>>> This property will allow the loop to be skipped.
>>>>>
>>>>> Signed-off-by: Sajid Dalvi <sdalvi@...gle.com>
>>>>> ---
>>>>
>>>> Thank you for your patch. There is something to discuss/improve.
>>>>
>>>>>  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 8 ++++++++
>>>>>  1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
>>>>> index 1a83f0f65f19..0b8950a73b7e 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
>>>>> @@ -197,6 +197,14 @@ properties:
>>>>>        - contains:
>>>>>            const: msi
>>>>>
>>>>> +  snps,skip-wait-link-up:
>>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>>> +    description:
>>>>> +      When the Root Complex is probed, the default behavior is to spin in a
>>>>> +      loop waiting for the link to come up. In some systems the link is not
>>>>> +      brought up during probe, but later in the context of an end-point turning
>>>>> +      on. This property will allow the loop to be skipped.
>>>>
>>>> I fail to see how probe behavior is related to properties of hardware.
>>>> You describe OS behavior, not hardware. This does not look like
>>>> belonging to DT.
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>> Thanks for your response Krzysztof.
>>> The hardware configuration of the system determines whether an
>>> endpoint device is available during host init. If it isn't available
>>> on a particular and dedicated pcie interface, we should skip waiting
>>> for the link to be up. For other interfaces, possibly even on the same
>>> system, where a device is present or maybe present we should wait for
>>> the link to come up.
>>
>> Keep discussions public.
>>
>> Your commit and property description mentions probe, which is nothing
>> related to hardware. Why the device would not be available during host
>> init (I understand we do not talk about hotplug as it is already
>> supported by Linux) in a way it is hardware property, not OS?
>>
>> Best regards,
>> Krzysztof
>>
> 
> + everyone else I mistakenly didn't reply to earlier
> 
> If I understand you correctly, the usage of probe is misleading
> because it doesn't have anything to do with the hardware.
> So your recommendation is to replace probe with device init, in the
> description of the property and the commit message?

No, I asked there a question for which we need answer.

device init is also OS task... You need to explain why this is a
property of hardware, not OS behavior.

Best regards,
Krzysztof