linux-kernel - Re: [PATCH v3 1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller

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: <20220622072042epcms2p478ff972391aa11a8827347b3469fd00c@epcms2p4>
Date:   Wed, 22 Jun 2022 16:20:42 +0900
From:   Wangseok Lee <wangseok.lee@...sung.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
        "kishon@...com" <kishon@...com>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jesper.nilsson@...s.com" <jesper.nilsson@...s.com>,
        "lars.persson@...s.com" <lars.persson@...s.com>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "kw@...ux.com" <kw@...ux.com>,
        "linux-arm-kernel@...s.com" <linux-arm-kernel@...s.com>,
        "kernel@...s.com" <kernel@...s.com>
CC:     Moon-Ki Jun <moonki.jun@...sung.com>,
        Sang Min Kim <hypmean.kim@...sung.com>,
        Dongjin Yang <dj76.yang@...sung.com>,
        Yeeun Kim <yeeun119.kim@...sung.com>
Subject: Re: [PATCH v3 1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller

On 21/06/2022 21:44, Krzysztof Kozlowski wrote:
> On 21/06/2022 09:42, Wangseok Lee wrote:
>>>>  
>>>>  samsung,syscon-bus-s-fsys:
>>>>    description:
>>>>      Phandle to bus-s path of fsys block, this register
>>>>      are used for enabling bus-s.
>>>>    $ref: /schemas/types.yaml#/definitions/phandle
>>>>  
>>>>  samsung,syscon-bus-p-fsys:
>>>>    description:
>>>>      Phandle to bus-p path of fsys block, this register
>>>>      are used for enabling bus-p.
>>>>    $ref: /schemas/types.yaml#/definitions/phandle
>>>
>>> This two look unspecific and hacky workaround for missing drivers. Looks
>>> like instead of implementing interconnect or clock driver, you decided
>>> to poke some other registers. Why this cannot be an interconnect driver?
>>>
>>>
>> 
>> bus-s, bus-p is a register that exists in the sysreg of the fsys block.
>> It is the same block as "fsys-sysreg" but is separated separately in
>> hardware.
> 
> Two points here:
> 1. If it is in FSYS, why it cannot be accessed with samsung,fsys-sysreg?
> 2. If it is only register, shuld be described like this. You must
> describe item:
> https://protect2.fireeye.com/v1/url?k=0f529a57-50c9a332-0f531118-000babff32e3-50938d8198077d59&q=1&e=32284e69-bbed-4d09-b6d6-0a43428aebf5&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.18-rc1%2Fsource%2FDocumentation%2Fdevicetree%2Fbindings%2Fsoc%2Fsamsung%2Fexynos-usi.yaml%23L42
> 

It would be better to access with fsys-sysreg, but their h/w address are
far from each other. The fsys block consists of a system register and an
additional control system register. "bus-s-fsys" and "bus-p-fsys" are
additional control system register. sysreg and additional control sysreg
addresses are far from each other and there are h/w registers that perform
different functions between them.

>> So, get resource is performed separately from "fsys-sysreg".
>> They set pcie slave, dbi related control settings,
>> naming "bus-x" seems to be interconnect.
>> I will add this description to property.
>> I don't think it need to use the interconnect driver,
>> so please let me know your opinion.
> 
> Please document both in the bindings and in the driver usage of this
> register. Writing there "0" or "1" is not enough. If the documentation
> is good, I am fine with it. If the explanation is obfuscated/not
> sufficient, it will look like avoiding to implement a driver, which I
> don't want to accept.
> 

I think i should add enough description. Is it sufficient to modify
the name and description of property like this?

samsung,fsys-bus-s:
  description:
    Phandle to bus-s of fsys block, this register
    is additional control sysreg in fsys block and
    this is used for pcie slave control setting.
  $ref: /schemas/types.yaml#/definitions/phandle

samsung,fsys-bus-p:
  description:
    Phandle to bus-p of fsys block, this register
    is additional control sysreg in fsys block and
    this is used for pcie dbi control setting.
  $ref: /schemas/types.yaml#/definitions/phandle

> Best regards,
> Krzysztof

Thank you for kindness reivew.

Best regards,
Wangseok Lee