Signed-off-by: Yang Xiwen <[email protected]>
---
Changes in v2:
- replace email.
- hisi-femac: s/BUS/MACIF (Andrew Lunn)
- hisi-femac: add "hisilicon,hisi-femac" compatible since the driver
seems generic enough for various SoCs
- hisi-femac-mdio: convert binding to YAML (Krzysztof Kozlowski)
- rewrite commit logs (Krzysztof Kozlowski)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Yang Xiwen (6):
net: hisilicon: add support for hisi_femac core on Hi3798MV200
net: hisi_femac: remove unused compatible strings
dt-bindings: net: remove outdated hisilicon-femac
dt-bindings: net: add hisilicon,hisi-femac
net: mdio: hisi-femac: make clock optional
dt-bindings: net: hisilicon-femac-mdio: convert to YAML
.../bindings/net/hisilicon,hisi-femac-mdio.yaml | 48 +++++++++
.../bindings/net/hisilicon,hisi-femac.yaml | 117 +++++++++++++++++++++
.../bindings/net/hisilicon-femac-mdio.txt | 22 ----
.../devicetree/bindings/net/hisilicon-femac.txt | 41 --------
drivers/net/ethernet/hisilicon/hisi_femac.c | 94 ++++++++++++-----
drivers/net/mdio/mdio-hisi-femac.c | 2 +-
6 files changed, 235 insertions(+), 89 deletions(-)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240216-net-9a208e17c40f
Best regards,
--
Yang Xiwen <[email protected]>
From: Yang Xiwen <[email protected]>
This binding gets rewritten. Compared to previous txt based binding doc,
the following changes are made according to the TRM:
- No "hisi-femac-v1/2" binding anymore
- Remove unmaintained Hi3516 SoC, add Hi3798MV200
- add MDIO subnode
- add ahb bus clock, phy clock and reset
Signed-off-by: Yang Xiwen <[email protected]>
---
.../bindings/net/hisilicon,hisi-femac.yaml | 117 +++++++++++++++++++++
1 file changed, 117 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml
new file mode 100644
index 000000000000..08158118c9c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml
@@ -0,0 +1,117 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hisilicon,hisi-femac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon Fast Ethernet MAC controller
+
+maintainers:
+ - Yang Xiwen <[email protected]>
+
+allOf:
+ - $ref: ethernet-controller.yaml
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - hisilicon,hi3798mv200-femac
+ - const: hisilicon,hisi-femac
+
+ reg:
+ items:
+ - description: The first region is the MAC core register base and size.
+ - description: The second region is the global MAC control register.
+
+ ranges:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: mac
+ - const: macif
+ - const: phy
+
+ resets:
+ maxItems: 2
+
+ reset-names:
+ items:
+ - const: mac
+ - const: phy
+
+ hisilicon,phy-reset-delays-us:
+ items:
+ - description: The 1st cell is reset pre-delay in micro seconds.
+ - description: The 2nd cell is reset pulse in micro seconds.
+ - description: The 3rd cell is reset post-delay in micro seconds.
+
+patternProperties:
+ '^mdio@[0-9a-f]+$':
+ $ref: hisilicon,hisi-femac-mdio.yaml#
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - phy-connection-type
+ - phy-handle
+ - hisilicon,phy-reset-delays-us
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/histb-clock.h>
+
+ ethernet@9c30000 {
+ compatible = "hisilicon,hi3798mv200-femac", "hisilicon,hisi-femac";
+ reg = <0x9c30000 0x1000>, <0x9c31300 0x200>;
+ ranges = <0x0 0x9c30000 0x10000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&crg HISTB_ETH0_MAC_CLK>,
+ <&crg HISTB_ETH0_MACIF_CLK>,
+ <&crg 62>;
+ clock-names = "mac", "macif", "phy";
+ resets = <&crg 0xd0 3>, <&crg 0x388 4>;
+ reset-names = "mac", "phy";
+ phy-handle = <&fephy>;
+ phy-connection-type = "mii";
+ // To be filled by bootloader
+ mac-address = [00 00 00 00 00 00];
+ hisilicon,phy-reset-delays-us = <10000 10000 500000>;
+ status = "okay";
+
+ mdio@1100 {
+ compatible = "hisilicon,hisi-femac-mdio";
+ reg = <0x1100 0x20>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ ethernet-phy@1 {
+ reg = <1>;
+ #phy-cells = <0>;
+ };
+ };
+ };
--
2.43.0
From: Yang Xiwen <[email protected]>
Considering that no users is found in the kernel, no backward
compatibility is maintained.
Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/net/ethernet/hisilicon/hisi_femac.c | 90 ++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
index 2406263c9dd3..15e90c7d6421 100644
--- a/drivers/net/ethernet/hisilicon/hisi_femac.c
+++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
@@ -10,8 +10,10 @@
#include <linux/etherdevice.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
@@ -97,6 +99,13 @@ enum phy_reset_delays {
DELAYS_NUM,
};
+enum clk_type {
+ CLK_MAC,
+ CLK_MACIF,
+ CLK_PHY,
+ CLK_NUM,
+};
+
struct hisi_femac_queue {
struct sk_buff **skb;
dma_addr_t *dma_phys;
@@ -108,7 +117,7 @@ struct hisi_femac_queue {
struct hisi_femac_priv {
void __iomem *port_base;
void __iomem *glb_base;
- struct clk *clk;
+ struct clk *clks[CLK_NUM];
struct reset_control *mac_rst;
struct reset_control *phy_rst;
u32 phy_reset_delays[DELAYS_NUM];
@@ -116,6 +125,7 @@ struct hisi_femac_priv {
struct device *dev;
struct net_device *ndev;
+ struct platform_device *mdio_pdev;
struct hisi_femac_queue txq;
struct hisi_femac_queue rxq;
@@ -693,6 +703,7 @@ static const struct net_device_ops hisi_femac_netdev_ops = {
static void hisi_femac_core_reset(struct hisi_femac_priv *priv)
{
reset_control_assert(priv->mac_rst);
+ usleep_range(200, 300);
reset_control_deassert(priv->mac_rst);
}
@@ -712,6 +723,10 @@ static void hisi_femac_sleep_us(u32 time_us)
static void hisi_femac_phy_reset(struct hisi_femac_priv *priv)
{
+ /* MAC clock must be disabled before PHY reset
+ */
+ clk_disable(priv->clks[CLK_MAC]);
+ clk_disable(priv->clks[CLK_MACIF]);
/* To make sure PHY hardware reset success,
* we must keep PHY in deassert state first and
* then complete the hardware reset operation
@@ -727,6 +742,9 @@ static void hisi_femac_phy_reset(struct hisi_femac_priv *priv)
reset_control_deassert(priv->phy_rst);
/* delay some time to ensure later MDIO access */
hisi_femac_sleep_us(priv->phy_reset_delays[POST_DELAY]);
+
+ clk_enable(priv->clks[CLK_MAC]);
+ clk_enable(priv->clks[CLK_MACIF]);
}
static void hisi_femac_port_init(struct hisi_femac_priv *priv)
@@ -768,11 +786,17 @@ static void hisi_femac_port_init(struct hisi_femac_priv *priv)
static int hisi_femac_drv_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct device_node *node = dev->of_node;
+ struct device_node *node = dev->of_node, *mdio_np;
struct net_device *ndev;
struct hisi_femac_priv *priv;
struct phy_device *phy;
- int ret;
+ int ret, i;
+ bool mdio_registered = false;
+ static const char * const clk_strs[] = {
+ [CLK_MAC] = "mac",
+ [CLK_MACIF] = "macif",
+ [CLK_PHY] = "phy",
+ };
ndev = alloc_etherdev(sizeof(*priv));
if (!ndev)
@@ -797,23 +821,20 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
goto out_free_netdev;
}
- priv->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(priv->clk)) {
- dev_err(dev, "failed to get clk\n");
- ret = -ENODEV;
- goto out_free_netdev;
- }
-
- ret = clk_prepare_enable(priv->clk);
- if (ret) {
- dev_err(dev, "failed to enable clk %d\n", ret);
- goto out_free_netdev;
+ for (i = 0; i < CLK_NUM; i++) {
+ priv->clks[i] = devm_clk_get_enabled(&pdev->dev, clk_strs[i]);
+ if (IS_ERR(priv->clks[i])) {
+ dev_err(dev, "failed to get enabled clk %s: %ld\n", clk_strs[i],
+ PTR_ERR(priv->clks[i]));
+ ret = -ENODEV;
+ goto out_free_netdev;
+ }
}
priv->mac_rst = devm_reset_control_get(dev, "mac");
if (IS_ERR(priv->mac_rst)) {
ret = PTR_ERR(priv->mac_rst);
- goto out_disable_clk;
+ goto out_free_netdev;
}
hisi_femac_core_reset(priv);
@@ -826,15 +847,32 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
priv->phy_reset_delays,
DELAYS_NUM);
if (ret)
- goto out_disable_clk;
+ goto out_free_netdev;
hisi_femac_phy_reset(priv);
}
+ // Register the optional MDIO bus
+ for_each_available_child_of_node(node, mdio_np) {
+ if (of_node_name_prefix(mdio_np, "mdio")) {
+ priv->mdio_pdev = of_platform_device_create(mdio_np, NULL, dev);
+ of_node_put(mdio_np);
+ if (!priv->mdio_pdev) {
+ dev_err(dev, "failed to register MDIO bus device\n");
+ goto out_free_netdev;
+ }
+ mdio_registered = true;
+ break;
+ }
+ }
+
+ if (!mdio_registered)
+ dev_warn(dev, "MDIO subnode notfound. This is usually a bug.\n");
+
phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link);
if (!phy) {
dev_err(dev, "connect to PHY failed!\n");
ret = -ENODEV;
- goto out_disable_clk;
+ goto out_unregister_mdio_bus;
}
phy_attached_print(phy, "phy_id=0x%.8lx, phy_mode=%s\n",
@@ -885,8 +923,8 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
out_disconnect_phy:
netif_napi_del(&priv->napi);
phy_disconnect(phy);
-out_disable_clk:
- clk_disable_unprepare(priv->clk);
+out_unregister_mdio_bus:
+ platform_device_unregister(priv->mdio_pdev);
out_free_netdev:
free_netdev(ndev);
@@ -897,12 +935,15 @@ static void hisi_femac_drv_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct hisi_femac_priv *priv = netdev_priv(ndev);
+ int i;
netif_napi_del(&priv->napi);
unregister_netdev(ndev);
phy_disconnect(ndev->phydev);
- clk_disable_unprepare(priv->clk);
+ platform_device_unregister(priv->mdio_pdev);
+ for (i = 0; i < CLK_NUM; i++)
+ clk_disable_unprepare(priv->clks[i]);
free_netdev(ndev);
}
@@ -912,6 +953,7 @@ static int hisi_femac_drv_suspend(struct platform_device *pdev,
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct hisi_femac_priv *priv = netdev_priv(ndev);
+ int i;
disable_irq(ndev->irq);
if (netif_running(ndev)) {
@@ -919,7 +961,8 @@ static int hisi_femac_drv_suspend(struct platform_device *pdev,
netif_device_detach(ndev);
}
- clk_disable_unprepare(priv->clk);
+ for (i = 0; i < CLK_NUM; i++)
+ clk_disable_unprepare(priv->clks[i]);
return 0;
}
@@ -928,8 +971,10 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct hisi_femac_priv *priv = netdev_priv(ndev);
+ int i;
- clk_prepare_enable(priv->clk);
+ for (i = 0; i < CLK_NUM; i++)
+ clk_prepare_enable(priv->clks[i]);
if (priv->phy_rst)
hisi_femac_phy_reset(priv);
@@ -948,6 +993,7 @@ static const struct of_device_id hisi_femac_match[] = {
{.compatible = "hisilicon,hisi-femac-v1",},
{.compatible = "hisilicon,hisi-femac-v2",},
{.compatible = "hisilicon,hi3516cv300-femac",},
+ {.compatible = "hisilicon,hi3798mv200-femac",},
{},
};
--
2.43.0
From: Yang Xiwen <[email protected]>
The clocks are optional.
Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/net/mdio/mdio-hisi-femac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mdio/mdio-hisi-femac.c b/drivers/net/mdio/mdio-hisi-femac.c
index 6703f626ee83..a2620b200381 100644
--- a/drivers/net/mdio/mdio-hisi-femac.c
+++ b/drivers/net/mdio/mdio-hisi-femac.c
@@ -93,7 +93,7 @@ static int hisi_femac_mdio_probe(struct platform_device *pdev)
goto err_out_free_mdiobus;
}
- data->clk = devm_clk_get(&pdev->dev, NULL);
+ data->clk = devm_clk_get_optional(&pdev->dev, NULL);
if (IS_ERR(data->clk)) {
ret = PTR_ERR(data->clk);
goto err_out_free_mdiobus;
--
2.43.0
From: Yang Xiwen <[email protected]>
Also mark the clock optional
Signed-off-by: Yang Xiwen <[email protected]>
---
.../bindings/net/hisilicon,hisi-femac-mdio.yaml | 48 ++++++++++++++++++++++
.../bindings/net/hisilicon-femac-mdio.txt | 22 ----------
2 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/hisilicon,hisi-femac-mdio.yaml b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac-mdio.yaml
new file mode 100644
index 000000000000..1c85e6e7f8f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac-mdio.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hisilicon,hisi-femac-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon FEMAC MDIO bus
+
+maintainers:
+ - Yang Xiwen <[email protected]>
+
+allOf:
+ - $ref: mdio.yaml#
+
+properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ compatible:
+ const: hisilicon,hisi-femac-mdio
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ mdio@10091100 {
+ compatible = "hisilicon,hisi-femac-mdio";
+ reg = <0x10091100 0x20>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy@1 {
+ reg = <1>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/net/hisilicon-femac-mdio.txt b/Documentation/devicetree/bindings/net/hisilicon-femac-mdio.txt
deleted file mode 100644
index 23a39a309d17..000000000000
--- a/Documentation/devicetree/bindings/net/hisilicon-femac-mdio.txt
+++ /dev/null
@@ -1,22 +0,0 @@
-Hisilicon Fast Ethernet MDIO Controller interface
-
-Required properties:
-- compatible: should be "hisilicon,hisi-femac-mdio".
-- reg: address and length of the register set for the device.
-- clocks: A phandle to the reference clock for this device.
-
-- PHY subnode: inherits from phy binding [1]
-[1] Documentation/devicetree/bindings/net/phy.txt
-
-Example:
-mdio: mdio@10091100 {
- compatible = "hisilicon,hisi-femac-mdio";
- reg = <0x10091100 0x10>;
- clocks = <&crg HI3516CV300_MDIO_CLK>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- phy0: phy@1 {
- reg = <1>;
- };
-};
--
2.43.0
From: Yang Xiwen <[email protected]>
The user documented(Hi3516) is not found in current kernel anymore. And
it seems this SoC has been EOL for a long time. While at it, Remove this
binding entirely due to recent driver changes.
Signed-off-by: Yang Xiwen <[email protected]>
---
.../devicetree/bindings/net/hisilicon-femac.txt | 41 ----------------------
1 file changed, 41 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/hisilicon-femac.txt b/Documentation/devicetree/bindings/net/hisilicon-femac.txt
deleted file mode 100644
index 5f96976f3cea..000000000000
--- a/Documentation/devicetree/bindings/net/hisilicon-femac.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-Hisilicon Fast Ethernet MAC controller
-
-Required properties:
-- compatible: should contain one of the following version strings:
- * "hisilicon,hisi-femac-v1"
- * "hisilicon,hisi-femac-v2"
- and the soc string "hisilicon,hi3516cv300-femac".
-- reg: specifies base physical address(s) and size of the device registers.
- The first region is the MAC core register base and size.
- The second region is the global MAC control register.
-- interrupts: should contain the MAC interrupt.
-- clocks: A phandle to the MAC main clock.
-- resets: should contain the phandle to the MAC reset signal(required) and
- the PHY reset signal(optional).
-- reset-names: should contain the reset signal name "mac"(required)
- and "phy"(optional).
-- phy-mode: see ethernet.txt [1].
-- phy-handle: see ethernet.txt [1].
-- hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given.
- The 1st cell is reset pre-delay in micro seconds.
- The 2nd cell is reset pulse in micro seconds.
- The 3rd cell is reset post-delay in micro seconds.
-
-The MAC address will be determined using the optional properties
-defined in ethernet.txt[1].
-
-[1] Documentation/devicetree/bindings/net/ethernet.txt
-
-Example:
- hisi_femac: ethernet@10090000 {
- compatible = "hisilicon,hi3516cv300-femac","hisilicon,hisi-femac-v2";
- reg = <0x10090000 0x1000>,<0x10091300 0x200>;
- interrupts = <12>;
- clocks = <&crg HI3518EV200_ETH_CLK>;
- resets = <&crg 0xec 0>,<&crg 0xec 3>;
- reset-names = "mac","phy";
- mac-address = [00 00 00 00 00 00];
- phy-mode = "mii";
- phy-handle = <&phy0>;
- hisilicon,phy-reset-delays-us = <10000 20000 20000>;
- };
--
2.43.0
On Fri, 16 Feb 2024 18:02:03 +0800, Yang Xiwen wrote:
> This binding gets rewritten. Compared to previous txt based binding doc,
> the following changes are made according to the TRM:
>
> - No "hisi-femac-v1/2" binding anymore
> - Remove unmaintained Hi3516 SoC, add Hi3798MV200
> - add MDIO subnode
> - add ahb bus clock, phy clock and reset
>
> Signed-off-by: Yang Xiwen <[email protected]>
> ---
> .../bindings/net/hisilicon,hisi-femac.yaml | 117 +++++++++++++++++++++
> 1 file changed, 117 insertions(+)
>
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/hisilicon,hisi-femac.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac-mdio.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: ethernet@9c30000: mdio@1100: False schema does not allow {'compatible': ['hisilicon,hisi-femac-mdio'], 'reg': [[4352, 32]], '#address-cells': [[1]], '#size-cells': [[0]], 'status': ['okay'], 'ethernet-phy@1': {'reg': [[1]], '#phy-cells': [[0]]}}
from schema $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac.yaml#
Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: /example-0/ethernet@9c30000/mdio@1100: failed to match any schema with compatible: ['hisilicon,hisi-femac-mdio']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
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.
On 2/16/2024 7:50 PM, Rob Herring wrote:
> On Fri, 16 Feb 2024 18:02:03 +0800, Yang Xiwen wrote:
>> This binding gets rewritten. Compared to previous txt based binding doc,
>> the following changes are made according to the TRM:
>>
>> - No "hisi-femac-v1/2" binding anymore
>> - Remove unmaintained Hi3516 SoC, add Hi3798MV200
>> - add MDIO subnode
>> - add ahb bus clock, phy clock and reset
>>
>> Signed-off-by: Yang Xiwen <[email protected]>
>> ---
>> .../bindings/net/hisilicon,hisi-femac.yaml | 117 +++++++++++++++++++++
>> 1 file changed, 117 insertions(+)
>>
> 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/hisilicon,hisi-femac.yaml:
> Error in referenced schema matching $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac-mdio.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: ethernet@9c30000: mdio@1100: False schema does not allow {'compatible': ['hisilicon,hisi-femac-mdio'], 'reg': [[4352, 32]], '#address-cells': [[1]], '#size-cells': [[0]], 'status': ['okay'], 'ethernet-phy@1': {'reg': [[1]], '#phy-cells': [[0]]}}
> from schema $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac.yaml#
> Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: /example-0/ethernet@9c30000/mdio@1100: failed to match any schema with compatible: ['hisilicon,hisi-femac-mdio']
Seems i forgot to rearrange these patches. Will fix in v3.
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> 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.
>
--
Regards,
Yang Xiwen
On 2/16/2024 9:37 PM, Andrew Lunn wrote:
> On Fri, Feb 16, 2024 at 06:01:59PM +0800, Yang Xiwen via B4 Relay wrote:
>> Signed-off-by: Yang Xiwen <[email protected]>
>> ---
>> Changes in v2:
>> - replace email.
>> - hisi-femac: s/BUS/MACIF (Andrew Lunn)
>> - hisi-femac: add "hisilicon,hisi-femac" compatible since the driver
>> seems generic enough for various SoCs
>> - hisi-femac-mdio: convert binding to YAML (Krzysztof Kozlowski)
>> - rewrite commit logs (Krzysztof Kozlowski)
>> - Link to v1: https://lore.kernel.org/r/[email protected]
> Generally, you wait for discussion to finish before posting a new
> version. Also, netdev requests you wait a minimum of 24 hours between
> versions.
>
> Having discussion happening on two different versions of a patchset
> at once just causes confusion.
Sorry for that, it's the first time i send netdev patches.
> Andrew
>
> ---
> pw-bot: cr
--
Regards,
Yang Xiwen
On Fri, Feb 16, 2024 at 09:38:31PM +0800, Yang Xiwen wrote:
> On 2/16/2024 9:37 PM, Andrew Lunn wrote:
> > On Fri, Feb 16, 2024 at 06:01:59PM +0800, Yang Xiwen via B4 Relay wrote:
> > > Signed-off-by: Yang Xiwen <[email protected]>
> > > ---
> > > Changes in v2:
> > > - replace email.
> > > - hisi-femac: s/BUS/MACIF (Andrew Lunn)
> > > - hisi-femac: add "hisilicon,hisi-femac" compatible since the driver
> > > seems generic enough for various SoCs
> > > - hisi-femac-mdio: convert binding to YAML (Krzysztof Kozlowski)
> > > - rewrite commit logs (Krzysztof Kozlowski)
> > > - Link to v1: https://lore.kernel.org/r/[email protected]
> > Generally, you wait for discussion to finish before posting a new
> > version. Also, netdev requests you wait a minimum of 24 hours between
> > versions.
> >
> > Having discussion happening on two different versions of a patchset
> > at once just causes confusion.
> Sorry for that, it's the first time i send netdev patches.
Reading this might help you:
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
Andrew
On Fri, Feb 16, 2024 at 06:01:59PM +0800, Yang Xiwen via B4 Relay wrote:
> Signed-off-by: Yang Xiwen <[email protected]>
> ---
> Changes in v2:
> - replace email.
> - hisi-femac: s/BUS/MACIF (Andrew Lunn)
> - hisi-femac: add "hisilicon,hisi-femac" compatible since the driver
> seems generic enough for various SoCs
> - hisi-femac-mdio: convert binding to YAML (Krzysztof Kozlowski)
> - rewrite commit logs (Krzysztof Kozlowski)
> - Link to v1: https://lore.kernel.org/r/[email protected]
Generally, you wait for discussion to finish before posting a new
version. Also, netdev requests you wait a minimum of 24 hours between
versions.
Having discussion happening on two different versions of a patchset
at once just causes confusion.
Andrew
---
pw-bot: cr
On Fri, Feb 16, 2024 at 07:53:05PM +0800, Yang Xiwen wrote:
> On 2/16/2024 7:50 PM, Rob Herring wrote:
> > On Fri, 16 Feb 2024 18:02:03 +0800, Yang Xiwen wrote:
> > > This binding gets rewritten. Compared to previous txt based binding doc,
> > > the following changes are made according to the TRM:
> > >
> > > - No "hisi-femac-v1/2" binding anymore
> > > - Remove unmaintained Hi3516 SoC, add Hi3798MV200
> > > - add MDIO subnode
> > > - add ahb bus clock, phy clock and reset
> > >
> > > Signed-off-by: Yang Xiwen <[email protected]>
> > > ---
> > > .../bindings/net/hisilicon,hisi-femac.yaml | 117 +++++++++++++++++++++
> > > 1 file changed, 117 insertions(+)
> > >
> > 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/hisilicon,hisi-femac.yaml:
> > Error in referenced schema matching $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac-mdio.yaml
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: ethernet@9c30000: mdio@1100: False schema does not allow {'compatible': ['hisilicon,hisi-femac-mdio'], 'reg': [[4352, 32]], '#address-cells': [[1]], '#size-cells': [[0]], 'status': ['okay'], 'ethernet-phy@1': {'reg': [[1]], '#phy-cells': [[0]]}}
> > from schema $id: http://devicetree.org/schemas/net/hisilicon,hisi-femac.yaml#
> > Documentation/devicetree/bindings/net/hisilicon,hisi-femac.example.dtb: /example-0/ethernet@9c30000/mdio@1100: failed to match any schema with compatible: ['hisilicon,hisi-femac-mdio']
> Seems i forgot to rearrange these patches. Will fix in v3.
Maybe you can also improve your build testing.
We expect each individual patch to build cleanly, otherwise you could
break git bisect. So it is good the build your patches one by one. You
could throw in the DT checks as well at each stage.
Andrew
On Fri, Feb 16, 2024 at 06:02:00PM +0800, Yang Xiwen via B4 Relay wrote:
..
> @@ -826,15 +847,32 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
> priv->phy_reset_delays,
> DELAYS_NUM);
> if (ret)
> - goto out_disable_clk;
> + goto out_free_netdev;
> hisi_femac_phy_reset(priv);
> }
>
> + // Register the optional MDIO bus
> + for_each_available_child_of_node(node, mdio_np) {
> + if (of_node_name_prefix(mdio_np, "mdio")) {
> + priv->mdio_pdev = of_platform_device_create(mdio_np, NULL, dev);
> + of_node_put(mdio_np);
> + if (!priv->mdio_pdev) {
> + dev_err(dev, "failed to register MDIO bus device\n");
> + goto out_free_netdev;
Hi Yang Xiwen,
out_free_netdev will return ret.
However, it seems that ret is uninitialised here.
Perhaps it should be set to a negative error value?
Flagged by Smatch.
> + }
> + mdio_registered = true;
> + break;
> + }
> + }
> +
> + if (!mdio_registered)
> + dev_warn(dev, "MDIO subnode notfound. This is usually a bug.\n");
> +
> phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link);
> if (!phy) {
> dev_err(dev, "connect to PHY failed!\n");
> ret = -ENODEV;
> - goto out_disable_clk;
> + goto out_unregister_mdio_bus;
> }
>
> phy_attached_print(phy, "phy_id=0x%.8lx, phy_mode=%s\n",
..
On 2/19/2024 11:53 PM, Simon Horman wrote:
> On Fri, Feb 16, 2024 at 06:02:00PM +0800, Yang Xiwen via B4 Relay wrote:
>
> ...
>
>> @@ -826,15 +847,32 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
>> priv->phy_reset_delays,
>> DELAYS_NUM);
>> if (ret)
>> - goto out_disable_clk;
>> + goto out_free_netdev;
>> hisi_femac_phy_reset(priv);
>> }
>>
>> + // Register the optional MDIO bus
>> + for_each_available_child_of_node(node, mdio_np) {
>> + if (of_node_name_prefix(mdio_np, "mdio")) {
>> + priv->mdio_pdev = of_platform_device_create(mdio_np, NULL, dev);
>> + of_node_put(mdio_np);
>> + if (!priv->mdio_pdev) {
>> + dev_err(dev, "failed to register MDIO bus device\n");
>> + goto out_free_netdev;
> Hi Yang Xiwen,
>
> out_free_netdev will return ret.
> However, it seems that ret is uninitialised here.
> Perhaps it should be set to a negative error value?
Oh, you are right. Will fix this in next version.
>
> Flagged by Smatch.
>
>> + }
>> + mdio_registered = true;
>> + break;
>> + }
>> + }
>> +
>> + if (!mdio_registered)
>> + dev_warn(dev, "MDIO subnode notfound. This is usually a bug.\n");
>> +
>> phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link);
>> if (!phy) {
>> dev_err(dev, "connect to PHY failed!\n");
>> ret = -ENODEV;
>> - goto out_disable_clk;
>> + goto out_unregister_mdio_bus;
>> }
>>
>> phy_attached_print(phy, "phy_id=0x%.8lx, phy_mode=%s\n",
> ...
--
Regards,
Yang Xiwen