linux-kernel - Re: [PATCH v3 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

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: <20220620083821epcms2p57a65984523a0f2a3815e4873e8bfc6df@epcms2p5>
Date:   Mon, 20 Jun 2022 17:38:21 +0900
From:   Wangseok Lee <wangseok.lee@...sung.com>
To:     Krzysztof Kozlowski <krzk@...nel.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 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

On 17/06/2022 07:59, Krzysztof Kozlowski wrote:
> On 13/06/2022 18:29, Wangseok Lee wrote:
>> Add description to support Axis, ARTPEC-8 SoC.
>> ARTPEC-8 is the SoC platform of Axis Communications
>> and PCIe phy is designed based on SAMSUNG PHY.
> 
> No improvements here. On v2 I gave you link pointing to specific
> paragraph of our documentation which you need to apply - wrong wrapping.
> Is there something unclear here?
> 
> Please
> do
> not
> wrap
> in
> different
> style.
> 
> 

I think i misunderstood your review comment.
I will modify it as below.
s/SAMSUNG PHY/Samsung phy
Please let me know if there is anything I missed.

>> 
>> Signed-off-by: Wangseok Lee <wangseok.lee@...sung.com>
>> ---
>> v2->v3 :
>> -modify version history to fit the linux commit rule
>> -remove 'Device Tree Bindings' on title
>> -remove clock-names entries
>> -change node name to soc from artpec8 on excamples
>> 
>> v1->v2 :
>> -'make dt_binding_check' result improvement
>> -Add the missing property list
>> -Align the indentation of continued lines/entries
>> ---
>>  .../bindings/phy/axis,artpec8-pcie-phy.yaml        | 73 ++++++++++++++++++++++
>>  1 file changed, 73 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> new file mode 100644
>> index 0000000..316b774
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> @@ -0,0 +1,73 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=84f6a8c0-e57dbdd9-84f7238f-74fe485cbfec-e81bd79bfd1ff442&q=1&e=6739af06-730c-48db-aef8-026a9aaee1b4&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Faxis%2Cartpec8-pcie-phy.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=b4f1a4ba-d57ab1a3-b4f02ff5-74fe485cbfec-7f885cbe82dc7860&q=1&e=6739af06-730c-48db-aef8-026a9aaee1b4&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: ARTPEC-8 SoC PCIe PHY
>> +
>> +maintainers:
>> +  - Jesper Nilsson <jesper.nilsson@...s.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: axis,artpec8-pcie-phy
>> +
>> +  reg:
>> +    items:
>> +      - description: PHY registers.
>> +      - description: PHY coding sublayer registers.
>> +
>> +  reg-names:
>> +    items:
>> +      - const: phy
>> +      - const: pcs
>> +
>> +  "#phy-cells":
>> +    const: 0
>> +
>> +  clocks:
>> +    items:
>> +      - description: PCIe PHY reference clock

refer to sample-schema.yaml, even if the clock item is single,
it seems to be used as follows.

clocks:
  maxItems: 1

clock-names:
  items:
    - const: ref

If only "clocks:" are define and clock-names are not define,
the following warning occurs.
"'clock-names' does not match any of the regexes"

>> +
>> +  num-lanes:
>> +    const: 2
>> +
>> +  lcpll-ref-clk:
>> +    const: 1
> 
> Unknown field... custom properties need vendor (axis,), type (boolean)
> and description.
> 

"lcpl-ref-clk" has an enum type value, so i will modify it as below.


axis,lcpll-ref-clk:
  description:
    select the reference clock of phy and initialization is performed
    with the reference clock according to the selected value.
  $ref: /schemas/types.yaml#/definitions/uint32
  enum: [ 0, 1, 2, 3, 4 ]

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - "#phy-cells"
>> +  - clocks
>> +  - clock-names
>> +  - samsung,fsys-sysreg
> 
> Same problem as in patch #1.
> 

I will add the following items to the property.

samsung,fsys-sysreg:
  description:
    Phandle to system register of fsys block.
  $ref: /schemas/types.yaml#/definitions/phandle

>> +  - num-lanes
>> +  - lcpll-ref-clk
>> +
>> +additionalProperties: true
> 
> No, this must be false.
> 

yes, i miss it. I will fix this.

> Best regards,
> Krzysztof

Thank you for kindness reivew.

Best regards,
Wangseok Lee