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-next>] [day] [month] [year] [list]

Date:   Fri, 24 Feb 2023 16:09:51 -0600
From:   Sajid Dalvi <sdalvi@...gle.com>
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 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?

Sajid