Message ID | 20231115032515.4249-10-quic_luoj@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:a59:b0:164:83eb:24d7 with SMTP id 25csp2363134rwb; Tue, 14 Nov 2023 19:27:14 -0800 (PST) X-Google-Smtp-Source: AGHT+IEnOKeGwvgzSkWPLTNaJFbB3+MEWjwMiRITqTvCcTet9bN/OAhVc+HfZW3AJsVzbWqAtilU X-Received: by 2002:a17:90b:1bca:b0:281:416e:1c3f with SMTP id oa10-20020a17090b1bca00b00281416e1c3fmr10655986pjb.28.1700018833767; Tue, 14 Nov 2023 19:27:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700018833; cv=none; d=google.com; s=arc-20160816; b=CxD5NfE0rENghjkULjGWE4UPQ3U1La64r9qQltdNh2fM7Ab1y6UmGgLtUavRL/hMGj pTC2iQNbiIILb3oW+oaPoOy6Bku5lHBH4F+kN7OcmOjj9MlrQ0Clz/lcGGF8atCYNLah uXvN6EmSz/5LB+K7dchRbPktb34b+82Xml6uACcScL8QtGOn7IYb4K98Viio6D41in8d dLKsPSbUDawPlOFYKQ3diSZ+QMoztDfHa1HV+FbZW4/SQJRNoIM5eBNwKmOTKw5vNIvC HN5i9QtWWMHol1ti1gBE5TWVlUhLZ0nm449x7BqpJDcGXpraXu7CnoLMvBdyKwLNFodA SGCg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=AQpWJyQavn2ZQqxdC8CNOkUtvM8ck18GRwWMCErMJ1s=; fh=+g5tGayWdOm7NVf+9R7F5Bp+AAhjebsKXKT3IUU1ti0=; b=UNEqdJ74WE/wFg3w18+PIuE5iOzhZR59Ury6pfpwc8+l1PqSC4quU+3xAY9HtM0TTu TOaI0dJn2CwWzb6D2Ys0b+RlOkrryQCpnEOVCMCwUq0caJjZUb443dRlupwAhNN5t+b1 CKIyfu03P+pWAVy97v62ADqjrALWTxSp7/57ElPBGtHE6Gjcl2yB7MehZWN5uJINiYUO TNmZBOCjVqtj8RqHzHDgcRSd8/VTq7nDAQpaShKIV4ND7WFWDrkq2Sm8Rlq191laQrQW PrmXHRDa+2R19/ASrXSCEBWXcgHSr9DfdhL/cS15Grg7w9C0Zfffwh2tfOovt/QkLaIx 2i3Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=YiRYMVtS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id q3-20020a17090a2dc300b0027766994586si13714278pjm.71.2023.11.14.19.27.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 19:27:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=YiRYMVtS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 4A2FD80E65BB; Tue, 14 Nov 2023 19:26:57 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234475AbjKOD0s (ORCPT <rfc822;heyuhang3455@gmail.com> + 28 others); Tue, 14 Nov 2023 22:26:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47424 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234424AbjKOD0f (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 14 Nov 2023 22:26:35 -0500 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CBF57C3; Tue, 14 Nov 2023 19:26:21 -0800 (PST) Received: from pps.filterd (m0279864.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3AENg479027751; Wed, 15 Nov 2023 03:26:09 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=qcppdkim1; bh=AQpWJyQavn2ZQqxdC8CNOkUtvM8ck18GRwWMCErMJ1s=; b=YiRYMVtSm1bOe8Z6f604G93v+UtgZ67e6IdLQRiqpqS+cq5dlRCpxHQJWVj0hLTViqzJ /qCQ573My7SLvgBUeiNPo9buCInrQaIQ+L4z92q1CMt/Wm3v6R36swlsiu++mVkK7cvq VtR1RRmRkqDSC5m/e2O+J438udcT0U8hxSjQZNF4AjDMhT3mXm4cZgF98hooYYh7cQ6L GxuoRtZayeZe5ocrSZTM26HuBUF/c/ft8ZYs0G9MwCRAq2B50SIZumrno4yZQITsP3ki Int7NfFh+feh77Iq6c/XoXjIAAl7FxbpXmWg99zq2CLFF0LJYmvjK1rbFtxKl87/22fT 4Q== Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3ucfke0tmm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 15 Nov 2023 03:26:09 +0000 Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3AF3Q8so027100 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 15 Nov 2023 03:26:08 GMT Received: from akronite-sh-dev02.qualcomm.com (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.39; Tue, 14 Nov 2023 19:26:03 -0800 From: Luo Jie <quic_luoj@quicinc.com> To: <agross@kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <andrew@lunn.ch>, <hkallweit1@gmail.com>, <linux@armlinux.org.uk>, <robert.marko@sartura.hr> CC: <linux-arm-msm@vger.kernel.org>, <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <quic_srichara@quicinc.com> Subject: [PATCH 9/9] dt-bindings: net: ipq4019-mdio: Document ipq5332 platform Date: Wed, 15 Nov 2023 11:25:15 +0800 Message-ID: <20231115032515.4249-10-quic_luoj@quicinc.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231115032515.4249-1-quic_luoj@quicinc.com> References: <20231115032515.4249-1-quic_luoj@quicinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: bOk5oRJxc54QUcI3xmGN9Nx9uw3jSauR X-Proofpoint-ORIG-GUID: bOk5oRJxc54QUcI3xmGN9Nx9uw3jSauR X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-15_01,2023-11-14_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 adultscore=0 phishscore=0 clxscore=1015 bulkscore=0 priorityscore=1501 spamscore=0 mlxscore=0 suspectscore=0 mlxlogscore=999 malwarescore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2311150027 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 14 Nov 2023 19:26:57 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782598948372653573 X-GMAIL-MSGID: 1782598948372653573 |
Series |
add MDIO changes on ipq5332 platform
|
|
Commit Message
Jie Luo
Nov. 15, 2023, 3:25 a.m. UTC
On platform IPQ5332, the MDIO address of qca8084 can be programed
when the device tree property "fixup" defined, the clock sequence
needs to be completed before the PHY probeable.
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
.../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++-
1 file changed, 130 insertions(+), 8 deletions(-)
Comments
On Wed, 15 Nov 2023 11:25:15 +0800, Luo Jie wrote: > On platform IPQ5332, the MDIO address of qca8084 can be programed > when the device tree property "fixup" defined, the clock sequence > needs to be completed before the PHY probeable. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > .../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++- > 1 file changed, 130 insertions(+), 8 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: phyaddr-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: pcsaddr-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: mdio-clk-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: fixup: missing type definition Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/pcsphy0@5: failed to match any schema with compatible: ['qcom,qca8k_pcs'] Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/pcsphy1@6: failed to match any schema with compatible: ['qcom,qca8k_pcs'] Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/xpcsphy@7: failed to match any schema with compatible: ['qcom,qca8k_pcs'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231115032515.4249-10-quic_luoj@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
> + phy-reset-gpio: > + minItems: 1 > + maxItems: 3 > + description: > + GPIO used to reset the PHY, each GPIO is for resetting the connected > + ethernet PHY device. This is a PHY property, so should be in the PHY node. phylib should then take care of fit. > + > + phyaddr-fixup: > + description: Register address for programing MDIO address of PHY devices Please give a better description of this and the next two properties. > + > + pcsaddr-fixup: > + description: Register address for programing MDIO address of PCS devices > + > + mdio-clk-fixup: > + description: The initialization clocks to be configured > + > + fixup: > + description: The MDIO address of PHY/PCS device to be programed > + clocks: > + items: > + - description: MDIO clock source frequency fixed to 100MHZ > + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ > + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ > + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ > + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ The clock frequencies is not relevent here, the driver sets that up. Andrew
On 11/15/2023 10:35 PM, Andrew Lunn wrote: >> + phy-reset-gpio: >> + minItems: 1 >> + maxItems: 3 >> + description: >> + GPIO used to reset the PHY, each GPIO is for resetting the connected >> + ethernet PHY device. > > This is a PHY property, so should be in the PHY node. phylib should > then take care of fit. will check this and update in the next patch set. > >> + >> + phyaddr-fixup: >> + description: Register address for programing MDIO address of PHY devices > > Please give a better description of this and the next two properties. these fixup flags are for customizing the MDIO address of qca8084 PHY & PCS and doing the initialization configurations to bring up PHY. will describe it more detail in the next patch set. > >> + >> + pcsaddr-fixup: >> + description: Register address for programing MDIO address of PCS devices >> + >> + mdio-clk-fixup: >> + description: The initialization clocks to be configured >> + >> + fixup: >> + description: The MDIO address of PHY/PCS device to be programed >> + clocks: >> + items: >> + - description: MDIO clock source frequency fixed to 100MHZ >> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ >> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ >> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ >> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ > > The clock frequencies is not relevent here, the driver sets that up. OK, will remove the clock frequency values in the next patch set. > > Andrew
On 15/11/2023 04:25, Luo Jie wrote: > On platform IPQ5332, the MDIO address of qca8084 can be programed > when the device tree property "fixup" defined, the clock sequence > needs to be completed before the PHY probeable. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > .../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++- > 1 file changed, 130 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > index 3407e909e8a7..7ff92be14ee1 100644 > --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > @@ -15,11 +15,13 @@ properties: > - enum: > - qcom,ipq4019-mdio > - qcom,ipq5018-mdio > + - qcom,ipq5332-mdio > > - items: > - enum: > - qcom,ipq6018-mdio > - qcom,ipq8074-mdio > + - qcom,ipq9574-mdio > - const: qcom,ipq4019-mdio > > "#address-cells": > @@ -30,19 +32,47 @@ properties: > > reg: > minItems: 1 > - maxItems: 2 > + maxItems: 5 > description: > - the first Address and length of the register set for the MDIO controller. > - the second Address and length of the register for ethernet LDO, this second > - address range is only required by the platform IPQ50xx. > + the first Address and length of the register set for the MDIO controller, > + the optional second, third and fourth address and length of the register > + for ethernet LDO, these three address range are required by the platform > + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to > + select the reference clock. > + > + reg-names: > + minItems: 1 > + maxItems: 5 You must describe the items and constrain them per each variant. > > clocks: > - items: > - - description: MDIO clock source frequency fixed to 100MHZ > + minItems: 1 > + maxItems: 5 > + description: Doesn't this make all other variants with incorrect constraints? > + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy > + clocks enabled for resetting ethernet PHY. > > clock-names: > - items: > - - const: gcc_mdio_ahb_clk > + minItems: 1 > + maxItems: 5 > + > + phy-reset-gpio: No, for multiple reasons. It's gpios first of all. Where do you see such property? Where is the existing definition? Then it is "reset-gpios" if this is MDIO. Why do you put phy properties in MDIO? > + minItems: 1 > + maxItems: 3 > + description: > + GPIO used to reset the PHY, each GPIO is for resetting the connected > + ethernet PHY device. > + > + phyaddr-fixup: > + description: Register address for programing MDIO address of PHY devices You did not test code which you sent. > + > + pcsaddr-fixup: > + description: Register address for programing MDIO address of PCS devices > + > + mdio-clk-fixup: > + description: The initialization clocks to be configured > + > + fixup: > + description: The MDIO address of PHY/PCS device to be programed Please do not send untested code. ... > + > + qca8kphy2: ethernet-phy@3 { > + reg = <3>; > + fixup; > + }; > + > + qca8kphy3: ethernet-phy@4 { > + reg = <4>; > + fixup; > + }; > + > + qca8kpcs0: pcsphy0@5 { > + compatible = "qcom,qca8k_pcs"; > + reg = <5>; > + fixup; > + }; Fix indentation. Best regards, Krzysztof
On 11/16/2023 7:56 PM, Krzysztof Kozlowski wrote: > On 15/11/2023 04:25, Luo Jie wrote: >> On platform IPQ5332, the MDIO address of qca8084 can be programed >> when the device tree property "fixup" defined, the clock sequence >> needs to be completed before the PHY probeable. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- >> .../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++- >> 1 file changed, 130 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> index 3407e909e8a7..7ff92be14ee1 100644 >> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> @@ -15,11 +15,13 @@ properties: >> - enum: >> - qcom,ipq4019-mdio >> - qcom,ipq5018-mdio >> + - qcom,ipq5332-mdio >> >> - items: >> - enum: >> - qcom,ipq6018-mdio >> - qcom,ipq8074-mdio >> + - qcom,ipq9574-mdio >> - const: qcom,ipq4019-mdio >> >> "#address-cells": >> @@ -30,19 +32,47 @@ properties: >> >> reg: >> minItems: 1 >> - maxItems: 2 >> + maxItems: 5 >> description: >> - the first Address and length of the register set for the MDIO controller. >> - the second Address and length of the register for ethernet LDO, this second >> - address range is only required by the platform IPQ50xx. >> + the first Address and length of the register set for the MDIO controller, >> + the optional second, third and fourth address and length of the register >> + for ethernet LDO, these three address range are required by the platform >> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to >> + select the reference clock. >> + >> + reg-names: >> + minItems: 1 >> + maxItems: 5 > > You must describe the items and constrain them per each variant. Ok, will describe these items one by one in the next patch set. > >> >> clocks: >> - items: >> - - description: MDIO clock source frequency fixed to 100MHZ >> + minItems: 1 >> + maxItems: 5 >> + description: > > Doesn't this make all other variants with incorrect constraints? There are 5 clock items, the first one is the legacy MDIO clock, the other clocks are new added for ipq5332 platform, will describe it more clearly in the next patch set. > >> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy >> + clocks enabled for resetting ethernet PHY. >> >> clock-names: >> - items: >> - - const: gcc_mdio_ahb_clk >> + minItems: 1 >> + maxItems: 5 >> + >> + phy-reset-gpio: > > No, for multiple reasons. It's gpios first of all. Where do you see such > property? Where is the existing definition? will remove this property, and update to use the exited PHY GPIO reset. > > Then it is "reset-gpios" if this is MDIO. Why do you put phy properties > in MDIO? > >> + minItems: 1 >> + maxItems: 3 >> + description: >> + GPIO used to reset the PHY, each GPIO is for resetting the connected >> + ethernet PHY device. >> + >> + phyaddr-fixup: >> + description: Register address for programing MDIO address of PHY devices > > You did not test code which you sent. Hi Krzysztof, This patch is passed with the following command in my workspace. i will upgrade and install yamllint to make sure there is no warning reported anymore. make dt_bg_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > >> + >> + pcsaddr-fixup: >> + description: Register address for programing MDIO address of PCS devices >> + >> + mdio-clk-fixup: >> + description: The initialization clocks to be configured >> + >> + fixup: >> + description: The MDIO address of PHY/PCS device to be programed > > Please do not send untested code. > Ok, will complete the full test before uploading the patch. > > ... > >> + >> + qca8kphy2: ethernet-phy@3 { >> + reg = <3>; >> + fixup; >> + }; >> + >> + qca8kphy3: ethernet-phy@4 { >> + reg = <4>; >> + fixup; >> + }; >> + >> + qca8kpcs0: pcsphy0@5 { >> + compatible = "qcom,qca8k_pcs"; >> + reg = <5>; >> + fixup; >> + }; > > Fix indentation. Will Fix it in the next patch set, thanks. > > Best regards, > Krzysztof >
On 17/11/2023 11:36, Jie Luo wrote: >>> clocks: >>> - items: >>> - - description: MDIO clock source frequency fixed to 100MHZ >>> + minItems: 1 >>> + maxItems: 5 >>> + description: >> >> Doesn't this make all other variants with incorrect constraints? > > There are 5 clock items, the first one is the legacy MDIO clock, the > other clocks are new added for ipq5332 platform, will describe it more > clearly in the next patch set. OTHER variants. Not this one. > >> >>> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy >>> + clocks enabled for resetting ethernet PHY. >>> >>> clock-names: >>> - items: >>> - - const: gcc_mdio_ahb_clk >>> + minItems: 1 >>> + maxItems: 5 >>> + >>> + phy-reset-gpio: >> >> No, for multiple reasons. It's gpios first of all. Where do you see such >> property? Where is the existing definition? > > will remove this property, and update to use the exited PHY GPIO reset. > >> >> Then it is "reset-gpios" if this is MDIO. Why do you put phy properties >> in MDIO? >> >>> + minItems: 1 >>> + maxItems: 3 >>> + description: >>> + GPIO used to reset the PHY, each GPIO is for resetting the connected >>> + ethernet PHY device. >>> + >>> + phyaddr-fixup: >>> + description: Register address for programing MDIO address of PHY devices >> >> You did not test code which you sent. > > Hi Krzysztof, > This patch is passed with the following command in my workspace. > i will upgrade and install yamllint to make sure there is no > warning reported anymore. > > make dt_bg_check No clue what's this, but no, I do not believe you tested it at all. It's not about yamllint. It's was not tested. Look at errors reported on mailing list. > DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml Best regards, Krzysztof
On 11/17/2023 6:40 PM, Krzysztof Kozlowski wrote: > On 17/11/2023 11:36, Jie Luo wrote: >>>> clocks: >>>> - items: >>>> - - description: MDIO clock source frequency fixed to 100MHZ >>>> + minItems: 1 >>>> + maxItems: 5 >>>> + description: >>> >>> Doesn't this make all other variants with incorrect constraints? >> >> There are 5 clock items, the first one is the legacy MDIO clock, the >> other clocks are new added for ipq5332 platform, will describe it more >> clearly in the next patch set. > > OTHER variants. Not this one. The change here is for the clock number added for the ipq5332 platform, the other platforms still use only legacy MDIO clock. so i add minItems and maxItems, i will check other .yaml files for the reference. > >> >>> >>>> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy >>>> + clocks enabled for resetting ethernet PHY. >>>> >>>> clock-names: >>>> - items: >>>> - - const: gcc_mdio_ahb_clk >>>> + minItems: 1 >>>> + maxItems: 5 >>>> + >>>> + phy-reset-gpio: >>> >>> No, for multiple reasons. It's gpios first of all. Where do you see such >>> property? Where is the existing definition? >> >> will remove this property, and update to use the exited PHY GPIO reset. >> >>> >>> Then it is "reset-gpios" if this is MDIO. Why do you put phy properties >>> in MDIO? >>> >>>> + minItems: 1 >>>> + maxItems: 3 >>>> + description: >>>> + GPIO used to reset the PHY, each GPIO is for resetting the connected >>>> + ethernet PHY device. >>>> + >>>> + phyaddr-fixup: >>>> + description: Register address for programing MDIO address of PHY devices >>> >>> You did not test code which you sent. >> >> Hi Krzysztof, >> This patch is passed with the following command in my workspace. >> i will upgrade and install yamllint to make sure there is no >> warning reported anymore. >> >> make dt_bg_check > > No clue what's this, but no, I do not believe you tested it at all. It's > not about yamllint. It's was not tested. Look at errors reported on > mailing list. > >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > Hi Krzysztof, Here is the output when i run the dt check with this patch applied in my workspace, md64 is the alias for compiling the arm64 platform. md64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml warning: python package 'yamllint' not installed, skipping CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dts DTC_CHK Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb > > Best regards, > Krzysztof >
On 17/11/2023 12:20, Jie Luo wrote: > > > On 11/17/2023 6:40 PM, Krzysztof Kozlowski wrote: >> On 17/11/2023 11:36, Jie Luo wrote: >>>>> clocks: >>>>> - items: >>>>> - - description: MDIO clock source frequency fixed to 100MHZ >>>>> + minItems: 1 >>>>> + maxItems: 5 >>>>> + description: >>>> >>>> Doesn't this make all other variants with incorrect constraints? >>> >>> There are 5 clock items, the first one is the legacy MDIO clock, the >>> other clocks are new added for ipq5332 platform, will describe it more >>> clearly in the next patch set. >> >> OTHER variants. Not this one. > > The change here is for the clock number added for the ipq5332 platform, > the other platforms still use only legacy MDIO clock. Then your patch is wrong as I said. You now affect other variants. I don't quite get your responses. Style of them suggests that you disagree, but you are not providing any relevant argument. > > so i add minItems and maxItems, i will check other .yaml files for the > reference. > >> >>> >>>> >>>>> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy >>>>> + clocks enabled for resetting ethernet PHY. >>>>> >>>>> clock-names: >>>>> - items: >>>>> - - const: gcc_mdio_ahb_clk >>>>> + minItems: 1 >>>>> + maxItems: 5 >>>>> + >>>>> + phy-reset-gpio: >>>> >>>> No, for multiple reasons. It's gpios first of all. Where do you see such >>>> property? Where is the existing definition? >>> >>> will remove this property, and update to use the exited PHY GPIO reset. >>> >>>> >>>> Then it is "reset-gpios" if this is MDIO. Why do you put phy properties >>>> in MDIO? >>>> >>>>> + minItems: 1 >>>>> + maxItems: 3 >>>>> + description: >>>>> + GPIO used to reset the PHY, each GPIO is for resetting the connected >>>>> + ethernet PHY device. >>>>> + >>>>> + phyaddr-fixup: >>>>> + description: Register address for programing MDIO address of PHY devices >>>> >>>> You did not test code which you sent. >>> >>> Hi Krzysztof, >>> This patch is passed with the following command in my workspace. >>> i will upgrade and install yamllint to make sure there is no >>> warning reported anymore. >>> >>> make dt_bg_check >> >> No clue what's this, but no, I do not believe you tested it at all. It's >> not about yamllint. It's was not tested. Look at errors reported on >> mailing list. >> >>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> > > Hi Krzysztof, > Here is the output when i run the dt check with this patch applied in my > workspace, md64 is the alias for compiling the arm64 platform. We still do not know your base and dtschema version. Best regards, Krzysztof
On 11/17/2023 8:43 PM, Krzysztof Kozlowski wrote: > On 17/11/2023 12:20, Jie Luo wrote: >> >> >> On 11/17/2023 6:40 PM, Krzysztof Kozlowski wrote: >>> On 17/11/2023 11:36, Jie Luo wrote: >>>>>> clocks: >>>>>> - items: >>>>>> - - description: MDIO clock source frequency fixed to 100MHZ >>>>>> + minItems: 1 >>>>>> + maxItems: 5 >>>>>> + description: >>>>> >>>>> Doesn't this make all other variants with incorrect constraints? >>>> >>>> There are 5 clock items, the first one is the legacy MDIO clock, the >>>> other clocks are new added for ipq5332 platform, will describe it more >>>> clearly in the next patch set. >>> >>> OTHER variants. Not this one. >> >> The change here is for the clock number added for the ipq5332 platform, >> the other platforms still use only legacy MDIO clock. > > Then your patch is wrong as I said. You now affect other variants. I > don't quite get your responses. Style of them suggests that you > disagree, but you are not providing any relevant argument. The clock arguments are provided in the later part as below. i will also provide more detail clock names for the new added clocks for the ipq5332 platform in description. - if: properties: compatible: contains: enum: - qcom,ipq5332-mdio then: properties: clocks: items: - description: MDIO clock source frequency fixed to 100MHZ - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ clock-names: items: - const: gcc_mdio_ahb_clk - const: gcc_uniphy0_ahb_clk - const: gcc_uniphy0_sys_clk - const: gcc_uniphy1_ahb_clk - const: gcc_uniphy1_sys_clk > >> >> so i add minItems and maxItems, i will check other .yaml files for the >> reference. >> >>> >>>> >>>>> >>>>>> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy >>>>>> + clocks enabled for resetting ethernet PHY. >>>>>> >>>>>> clock-names: >>>>>> - items: >>>>>> - - const: gcc_mdio_ahb_clk >>>>>> + minItems: 1 >>>>>> + maxItems: 5 >>>>>> + >>>>>> + phy-reset-gpio: >>>>> >>>>> No, for multiple reasons. It's gpios first of all. Where do you see such >>>>> property? Where is the existing definition? >>>> >>>> will remove this property, and update to use the exited PHY GPIO reset. >>>> >>>>> >>>>> Then it is "reset-gpios" if this is MDIO. Why do you put phy properties >>>>> in MDIO? >>>>> >>>>>> + minItems: 1 >>>>>> + maxItems: 3 >>>>>> + description: >>>>>> + GPIO used to reset the PHY, each GPIO is for resetting the connected >>>>>> + ethernet PHY device. >>>>>> + >>>>>> + phyaddr-fixup: >>>>>> + description: Register address for programing MDIO address of PHY devices >>>>> >>>>> You did not test code which you sent. >>>> >>>> Hi Krzysztof, >>>> This patch is passed with the following command in my workspace. >>>> i will upgrade and install yamllint to make sure there is no >>>> warning reported anymore. >>>> >>>> make dt_bg_check >>> >>> No clue what's this, but no, I do not believe you tested it at all. It's >>> not about yamllint. It's was not tested. Look at errors reported on >>> mailing list. >>> >>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >>> >> >> Hi Krzysztof, >> Here is the output when i run the dt check with this patch applied in my >> workspace, md64 is the alias for compiling the arm64 platform. > > We still do not know your base and dtschema version. The code base is the commit id: 5ba73bec5e7b0494da7fdca3e003d8b97fa932cd <Add linux-next specific files for 20231114> The dtschema version is as below. dt-doc-validate --version 2023.9 > > > Best regards, > Krzysztof >
> The clock arguments are provided in the later part as below. i will also > provide more detail clock names for the new added clocks for the ipq5332 > platform in description. > > - if: > > properties: > > compatible: > > contains: > > enum: > > - qcom,ipq5332-mdio > > then: > > properties: > > clocks: > > items: > > - description: MDIO clock source frequency fixed to 100MHZ > > - description: UNIPHY0 AHB clock source frequency fixed to > 100MHZ > - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ > - description: UNIPHY1 AHB clock source frequency fixed to > 100MHZ > - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ As i said before, the frequency of the clocks does not matter here. That appears to be the drivers problem. I assume every board design, with any sort of PHY, needs the same clock configuration? Andrew
On 11/18/2023 11:36 PM, Andrew Lunn wrote: >> The clock arguments are provided in the later part as below. i will also >> provide more detail clock names for the new added clocks for the ipq5332 >> platform in description. >> >> - if: >> >> properties: >> >> compatible: >> >> contains: >> >> enum: >> >> - qcom,ipq5332-mdio >> >> then: >> >> properties: >> >> clocks: >> >> items: >> >> - description: MDIO clock source frequency fixed to 100MHZ >> >> - description: UNIPHY0 AHB clock source frequency fixed to >> 100MHZ >> - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ >> - description: UNIPHY1 AHB clock source frequency fixed to >> 100MHZ >> - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ > > As i said before, the frequency of the clocks does not matter > here. That appears to be the drivers problem. I assume every board > design, with any sort of PHY, needs the same clock configuration? > > Andrew Yes, Andrew, no matter what kind of PHY is connected, these clocks are fix clocks, the clock rates are same as mentioned above, which are the SOC clock configurations.
diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml index 3407e909e8a7..7ff92be14ee1 100644 --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml @@ -15,11 +15,13 @@ properties: - enum: - qcom,ipq4019-mdio - qcom,ipq5018-mdio + - qcom,ipq5332-mdio - items: - enum: - qcom,ipq6018-mdio - qcom,ipq8074-mdio + - qcom,ipq9574-mdio - const: qcom,ipq4019-mdio "#address-cells": @@ -30,19 +32,47 @@ properties: reg: minItems: 1 - maxItems: 2 + maxItems: 5 description: - the first Address and length of the register set for the MDIO controller. - the second Address and length of the register for ethernet LDO, this second - address range is only required by the platform IPQ50xx. + the first Address and length of the register set for the MDIO controller, + the optional second, third and fourth address and length of the register + for ethernet LDO, these three address range are required by the platform + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to + select the reference clock. + + reg-names: + minItems: 1 + maxItems: 5 clocks: - items: - - description: MDIO clock source frequency fixed to 100MHZ + minItems: 1 + maxItems: 5 + description: + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy + clocks enabled for resetting ethernet PHY. clock-names: - items: - - const: gcc_mdio_ahb_clk + minItems: 1 + maxItems: 5 + + phy-reset-gpio: + minItems: 1 + maxItems: 3 + description: + GPIO used to reset the PHY, each GPIO is for resetting the connected + ethernet PHY device. + + phyaddr-fixup: + description: Register address for programing MDIO address of PHY devices + + pcsaddr-fixup: + description: Register address for programing MDIO address of PCS devices + + mdio-clk-fixup: + description: The initialization clocks to be configured + + fixup: + description: The MDIO address of PHY/PCS device to be programed required: - compatible @@ -61,6 +91,8 @@ allOf: - qcom,ipq5018-mdio - qcom,ipq6018-mdio - qcom,ipq8074-mdio + - qcom,ipq5332-mdio + - qcom,ipq9574-mdio then: required: - clocks @@ -70,6 +102,29 @@ allOf: clocks: false clock-names: false + - if: + properties: + compatible: + contains: + enum: + - qcom,ipq5332-mdio + then: + properties: + clocks: + items: + - description: MDIO clock source frequency fixed to 100MHZ + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ + clock-names: + items: + - const: gcc_mdio_ahb_clk + - const: gcc_uniphy0_ahb_clk + - const: gcc_uniphy0_sys_clk + - const: gcc_uniphy1_ahb_clk + - const: gcc_uniphy1_sys_clk + unevaluatedProperties: false examples: @@ -100,3 +155,70 @@ examples: reg = <4>; }; }; + + - | + #include <dt-bindings/clock/qcom,ipq5332-gcc.h> + #include <dt-bindings/gpio/gpio.h> + + mdio@90000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "qcom,ipq5332-mdio"; + reg = <0x90000 0x64>, <0x7A00610 0x4>, <0x7A10610 0x4>, <0x9B000 0x800>; + reg-names = "mdio", "eth_ldo1", "eth_ldo2", "cmn_blk"; + + clocks = <&gcc GCC_MDIO_AHB_CLK>, + <&gcc GCC_UNIPHY0_AHB_CLK>, + <&gcc GCC_UNIPHY0_SYS_CLK>, + <&gcc GCC_UNIPHY1_AHB_CLK>, + <&gcc GCC_UNIPHY1_SYS_CLK>; + + clock-names = "gcc_mdio_ahb_clk", + "gcc_uniphy0_ahb_clk", + "gcc_uniphy0_sys_clk", + "gcc_uniphy1_ahb_clk", + "gcc_uniphy1_sys_clk"; + + phy-reset-gpio = <&tlmm 51 GPIO_ACTIVE_LOW>; + phyaddr-fixup = <0xC90F018>; + pcsaddr-fixup = <0xC90F014>; + mdio-clk-fixup; + + qca8kphy0: ethernet-phy@1 { + reg = <1>; + fixup; + }; + + qca8kphy1: ethernet-phy@2 { + reg = <2>; + fixup; + }; + + qca8kphy2: ethernet-phy@3 { + reg = <3>; + fixup; + }; + + qca8kphy3: ethernet-phy@4 { + reg = <4>; + fixup; + }; + + qca8kpcs0: pcsphy0@5 { + compatible = "qcom,qca8k_pcs"; + reg = <5>; + fixup; + }; + + qca8kpcs1: pcsphy1@6 { + compatible = "qcom,qca8k_pcs"; + reg = <6>; + fixup; + }; + + qca8kxpcs: xpcsphy@7 { + compatible = "qcom,qca8k_pcs"; + reg = <7>; + fixup; + }; + };