linux-kernel - Re: [PATCH v3 1/2] dt-bindings: phy: ti: phy-gmii-sel: Add bindings for J7200

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]

Date:   Tue, 23 Aug 2022 14:57:53 +0530
From:   Siddharth Vadapalli <s-vadapalli@...com>
To:     Rob Herring <robh@...nel.org>
CC:     <lee.jones@...aro.org>, <krzysztof.kozlowski@...aro.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <kishon@...com>,
        <vkoul@...nel.org>, <dan.carpenter@...cle.com>,
        <grygorii.strashko@...com>, <rogerq@...nel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-phy@...ts.infradead.org>, <s-vadapalli@...com>
Subject: Re: [PATCH v3 1/2] dt-bindings: phy: ti: phy-gmii-sel: Add bindings
 for J7200

Hello Rob,

On 23/08/22 03:11, Rob Herring wrote:
> On Mon, Aug 22, 2022 at 12:26:30PM +0530, Siddharth Vadapalli wrote:
>> TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
>> that are not supported on earlier SoCs. Add a compatible for it.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
>> ---
>>  .../mfd/ti,j721e-system-controller.yaml       |  6 ++++
>>  .../bindings/phy/ti,phy-gmii-sel.yaml         | 30 ++++++++++++++++++-
>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> index 73cffc45e056..466724cb4157 100644
>> --- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> @@ -54,6 +54,12 @@ patternProperties:
>>      description:
>>        Clock provider for TI EHRPWM nodes.
>>  
>> +  "phy@[0-9a-f]+$":
>> +    type: object
>> +    $ref: ../phy/ti,phy-gmii-sel.yaml
> 
> /schemas/phy/...

Thank you for reviewing the patch. I will update $ref to
/schemas/phy/phy-provider.yaml.

> 
>> +    description:
>> +      This is the register to set phy mode through phy-gmii-sel driver.
>> +
>>  required:
>>    - compatible
>>    - reg
>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> index ff8a6d9eb153..0ffb97f1a77c 100644
>> --- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> @@ -53,12 +53,24 @@ properties:
>>        - ti,am43xx-phy-gmii-sel
>>        - ti,dm814-phy-gmii-sel
>>        - ti,am654-phy-gmii-sel
>> +      - ti,j7200-cpsw5g-phy-gmii-sel
>>  
>>    reg:
>>      maxItems: 1
>>  
>>    '#phy-cells': true
>>  
>> +  ti,qsgmii-main-ports:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description: |
>> +      Required only for QSGMII mode. Array to select the port for
>> +      QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
>> +      ports automatically. Any one of the 4 CPSW5G ports can act as the
>> +      main port with the rest of them being the QSGMII_SUB ports.
>> +    items:
>> +      minimum: 1
>> +      maximum: 4
>> +
>>  allOf:
>>    - if:
>>        properties:
>> @@ -73,6 +85,22 @@ allOf:
>>          '#phy-cells':
>>            const: 1
>>            description: CPSW port number (starting from 1)
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - ti,j7200-cpsw5g-phy-gmii-sel
>> +    then:
>> +      properties:
>> +        '#phy-cells':
>> +          const: 1
>> +          description: CPSW port number (starting from 1)
>> +        ti,qsgmii-main-ports:
>> +          maxItems: 1
> 
> If the array size can only ever be 1, then it's a uint32, not a 
> uint32-array.

For the current device, there can be only one QSGMII main port and
uint32 will be sufficient. However, I plan to send patches for TI's
J721e device which supports up to 2 QSGMII main ports. I wish to
implement the property such that it can be reused by J721e. For this
reason, I am defining the property as a uint32-array. With this
implementation, J7200 will use the property as an array with one
element, while J721e will use the property as an array with two
elements. This is being done to avoid adding a new property in the
future just for J721e. Please let me know if this is fine.

Regards,
Siddharth.