| Message ID | 20240216-net-v1-1-e0ad972cda99@outlook.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: hisi-femac: add support for Hi3798MV200, remove unmaintained compatibles | expand |
> + 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; > + } The clk API has devm_clk_bulk_ versions. Please take a look at them, and see if it will simplify the code. Andrew
On 2/16/2024 7:57 AM, Andrew Lunn wrote: >> + 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; >> + } > The clk API has devm_clk_bulk_ versions. Please take a look at them, and see > if it will simplify the code. I know this API, but it can't be used. We need to control clocks individually in reset procedure. > Andrew
> + // 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"); I don't understand the architecture of this device yet... It seems like you have an integrated PHY? In the example, you used a phy-handle to bind the MAC to the PHY. So why is the MDIO bus optional? Do the MII signals from the MAC also go to SoC pins, so you could use an external PHY? Is there a SERDES so you could connect to an SFP cage? Also, do the MDIO pins go to SoC pins? Can the MDIO bus master be used to control external PHYs? If everything is internal, fixed in silicon, no variation possible, you don't need to describe the MDIO bus in DT. The MAC driver can register it, and then get the PHY at the hard coded address it uses. Andrew
On 2/16/2024 9:23 PM, Andrew Lunn wrote: >> + // 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"); > I don't understand the architecture of this device yet... > > It seems like you have an integrated PHY? In the example, you used a > phy-handle to bind the MAC to the PHY. So why is the MDIO bus > optional? Because the MAC can also support external PHY according to the datasheet. Maybe some other SoCs didn't implement this internal PHY and used an external PHY instead. > > Do the MII signals from the MAC also go to SoC pins, so you could use > an external PHY? Is there a SERDES so you could connect to an SFP > cage? No. MII signals is not accessible outside of the SoC. The SoC only exports FEPHY pins (i.e. RXN(P) and TXN(P)). > > Also, do the MDIO pins go to SoC pins? Can the MDIO bus master be used > to control external PHYs? It can, but not for Hi3798MV200. The datasheet said it can use both internal phy or external phy. But for Hi3798MV200, seems impossible. > > If everything is internal, fixed in silicon, no variation possible, > you don't need to describe the MDIO bus in DT. The MAC driver can > register it, and then get the PHY at the hard coded address it uses. > > Andrew
On Fri, Feb 16, 2024 at 07:59:19AM +0800, Yang Xiwen wrote: > On 2/16/2024 7:57 AM, Andrew Lunn wrote: > > > + 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; > > > + } > > The clk API has devm_clk_bulk_ versions. Please take a look at them, and see > > if it will simplify the code. > I know this API, but it can't be used. We need to control clocks > individually in reset procedure. /** * struct clk_bulk_data - Data used for bulk clk operations. * * @id: clock consumer ID * @clk: struct clk * to store the associated clock * * The CLK APIs provide a series of clk_bulk_() API calls as * a convenience to consumers which require multiple clks. This * structure is used to manage data for these calls. */ struct clk_bulk_data { const char *id; struct clk *clk; }; You pass the bulk API calls an array of this structure. After the get has completed, you can access the individual clocks via the clk pointer. So you can use the bulk API on probe and remove when you need to operate on all three, and the single clk API for your reset handler etc. Andrew
On 2/16/2024 9:49 PM, Andrew Lunn wrote: > On Fri, Feb 16, 2024 at 07:59:19AM +0800, Yang Xiwen wrote: >> On 2/16/2024 7:57 AM, Andrew Lunn wrote: >>>> + 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; >>>> + } >>> The clk API has devm_clk_bulk_ versions. Please take a look at them, and see >>> if it will simplify the code. >> I know this API, but it can't be used. We need to control clocks >> individually in reset procedure. > /** > * struct clk_bulk_data - Data used for bulk clk operations. > * > * @id: clock consumer ID > * @clk: struct clk * to store the associated clock > * > * The CLK APIs provide a series of clk_bulk_() API calls as > * a convenience to consumers which require multiple clks. This > * structure is used to manage data for these calls. > */ > struct clk_bulk_data { > const char *id; > struct clk *clk; > }; > > You pass the bulk API calls an array of this structure. After the get > has completed, you can access the individual clocks via the clk > pointer. So you can use the bulk API on probe and remove when you need > to operate on all three, and the single clk API for your reset handler > etc. seems okay. I'll implement this in next version. > > Andrew
On Fri, Feb 16, 2024 at 09:41:29PM +0800, Yang Xiwen wrote: > On 2/16/2024 9:23 PM, Andrew Lunn wrote: > > > + // 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"); > > I don't understand the architecture of this device yet... > > > > It seems like you have an integrated PHY? In the example, you used a > > phy-handle to bind the MAC to the PHY. So why is the MDIO bus > > optional? > Because the MAC can also support external PHY according to the datasheet. > Maybe some other SoCs didn't implement this internal PHY and used an > external PHY instead. > > > > Do the MII signals from the MAC also go to SoC pins, so you could use > > an external PHY? Is there a SERDES so you could connect to an SFP > > cage? > No. MII signals is not accessible outside of the SoC. The SoC only exports > FEPHY pins (i.e. RXN(P) and TXN(P)). > > > > Also, do the MDIO pins go to SoC pins? Can the MDIO bus master be used > > to control external PHYs? > It can, but not for Hi3798MV200. The datasheet said it can use both internal > phy or external phy. But for Hi3798MV200, seems impossible. So for the Hi3798MV200 this is not optional, the MDIO bus is mandatory. Also, it sounds like it exists in the silicon. So it is better to always describe it in the .dtsi file. And i took a quick look at mdio-hisi-femac.c. It has a probe function which does: data->membase = devm_platform_ioremap_resource(pdev, 0); meaning it expects to have its own address range. It is a device of its own. That also explains the compatible. So please move the MDIO bus to a node of its own, rather than embedding it within the MAC node. Andrew
On 2/16/2024 9:58 PM, Andrew Lunn wrote: > On Fri, Feb 16, 2024 at 09:41:29PM +0800, Yang Xiwen wrote: >> On 2/16/2024 9:23 PM, Andrew Lunn wrote: >>>> + // 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"); >>> I don't understand the architecture of this device yet... >>> >>> It seems like you have an integrated PHY? In the example, you used a >>> phy-handle to bind the MAC to the PHY. So why is the MDIO bus >>> optional? >> Because the MAC can also support external PHY according to the datasheet. >> Maybe some other SoCs didn't implement this internal PHY and used an >> external PHY instead. >>> Do the MII signals from the MAC also go to SoC pins, so you could use >>> an external PHY? Is there a SERDES so you could connect to an SFP >>> cage? >> No. MII signals is not accessible outside of the SoC. The SoC only exports >> FEPHY pins (i.e. RXN(P) and TXN(P)). >>> Also, do the MDIO pins go to SoC pins? Can the MDIO bus master be used >>> to control external PHYs? >> It can, but not for Hi3798MV200. The datasheet said it can use both internal >> phy or external phy. But for Hi3798MV200, seems impossible. > So for the Hi3798MV200 this is not optional, the MDIO bus is > mandatory. > > Also, it sounds like it exists in the silicon. So it is better to > always describe it in the .dtsi file. > > And i took a quick look at mdio-hisi-femac.c. It has a probe function > which does: > > data->membase = devm_platform_ioremap_resource(pdev, 0); > > meaning it expects to have its own address range. It is a device of > its own. That also explains the compatible. So please move the MDIO > bus to a node of its own, rather than embedding it within the MAC > node. It won't work. Hi3798MV200 does not have a dedicated MDIO bus clock. this clock is merged to MAC clock for this SoC. We need to enable CLK_BUS and CLK_MAC before MDIO bus access. Assigning CLK_MAC and CLK_BUS to MDIO bus device does not work either. Because the clocks will be enabled twice, the MAC controller will be unable to disable the clocks during PHY reset. Note that the source is contributed by HiSilicon almost 8 yrs ago. And they had never proved that this driver works. They are not doing things like this in downstream. > > Andrew
Hi Yang,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/net-hisilicon-add-support-for-hisi_femac-core-on-Hi3798MV200/20240216-075015
base: 8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link: https://lore.kernel.org/r/20240216-net-v1-1-e0ad972cda99%40outlook.com
patch subject: [PATCH 1/6] net: hisilicon: add support for hisi_femac core on Hi3798MV200
config: arm-randconfig-r081-20240223 (https://download.01.org/0day-ci/archive/20240224/202402242354.leEII4M9-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project edd4aee4dd9b5b98b2576a6f783e4086173d902a)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202402242354.leEII4M9-lkp@intel.com/
smatch warnings:
drivers/net/ethernet/hisilicon/hisi_femac.c:931 hisi_femac_drv_probe() error: uninitialized symbol 'ret'.
vim +/ret +931 drivers/net/ethernet/hisilicon/hisi_femac.c
542ae60af24f02 Dongpo Li 2016-07-15 786 static int hisi_femac_drv_probe(struct platform_device *pdev)
542ae60af24f02 Dongpo Li 2016-07-15 787 {
542ae60af24f02 Dongpo Li 2016-07-15 788 struct device *dev = &pdev->dev;
3c2c758a1607ea Yang Xiwen 2024-02-16 789 struct device_node *node = dev->of_node, *mdio_np;
542ae60af24f02 Dongpo Li 2016-07-15 790 struct net_device *ndev;
542ae60af24f02 Dongpo Li 2016-07-15 791 struct hisi_femac_priv *priv;
542ae60af24f02 Dongpo Li 2016-07-15 792 struct phy_device *phy;
3c2c758a1607ea Yang Xiwen 2024-02-16 793 int ret, i;
3c2c758a1607ea Yang Xiwen 2024-02-16 794 bool mdio_registered = false;
3c2c758a1607ea Yang Xiwen 2024-02-16 795 static const char * const clk_strs[] = {
3c2c758a1607ea Yang Xiwen 2024-02-16 796 [CLK_MAC] = "mac",
3c2c758a1607ea Yang Xiwen 2024-02-16 797 [CLK_BUS] = "bus",
3c2c758a1607ea Yang Xiwen 2024-02-16 798 [CLK_PHY] = "phy",
3c2c758a1607ea Yang Xiwen 2024-02-16 799 };
542ae60af24f02 Dongpo Li 2016-07-15 800
542ae60af24f02 Dongpo Li 2016-07-15 801 ndev = alloc_etherdev(sizeof(*priv));
542ae60af24f02 Dongpo Li 2016-07-15 802 if (!ndev)
542ae60af24f02 Dongpo Li 2016-07-15 803 return -ENOMEM;
542ae60af24f02 Dongpo Li 2016-07-15 804
542ae60af24f02 Dongpo Li 2016-07-15 805 platform_set_drvdata(pdev, ndev);
2087d421a5a1af Dongpo Li 2016-12-12 806 SET_NETDEV_DEV(ndev, &pdev->dev);
542ae60af24f02 Dongpo Li 2016-07-15 807
542ae60af24f02 Dongpo Li 2016-07-15 808 priv = netdev_priv(ndev);
542ae60af24f02 Dongpo Li 2016-07-15 809 priv->dev = dev;
542ae60af24f02 Dongpo Li 2016-07-15 810 priv->ndev = ndev;
542ae60af24f02 Dongpo Li 2016-07-15 811
56170ba3bd9098 Jiangfeng Xiao 2019-07-12 812 priv->port_base = devm_platform_ioremap_resource(pdev, 0);
542ae60af24f02 Dongpo Li 2016-07-15 813 if (IS_ERR(priv->port_base)) {
542ae60af24f02 Dongpo Li 2016-07-15 814 ret = PTR_ERR(priv->port_base);
542ae60af24f02 Dongpo Li 2016-07-15 815 goto out_free_netdev;
542ae60af24f02 Dongpo Li 2016-07-15 816 }
542ae60af24f02 Dongpo Li 2016-07-15 817
56170ba3bd9098 Jiangfeng Xiao 2019-07-12 818 priv->glb_base = devm_platform_ioremap_resource(pdev, 1);
542ae60af24f02 Dongpo Li 2016-07-15 819 if (IS_ERR(priv->glb_base)) {
542ae60af24f02 Dongpo Li 2016-07-15 820 ret = PTR_ERR(priv->glb_base);
542ae60af24f02 Dongpo Li 2016-07-15 821 goto out_free_netdev;
542ae60af24f02 Dongpo Li 2016-07-15 822 }
542ae60af24f02 Dongpo Li 2016-07-15 823
3c2c758a1607ea Yang Xiwen 2024-02-16 824 for (i = 0; i < CLK_NUM; i++) {
3c2c758a1607ea Yang Xiwen 2024-02-16 825 priv->clks[i] = devm_clk_get_enabled(&pdev->dev, clk_strs[i]);
3c2c758a1607ea Yang Xiwen 2024-02-16 826 if (IS_ERR(priv->clks[i])) {
3c2c758a1607ea Yang Xiwen 2024-02-16 827 dev_err(dev, "failed to get enabled clk %s: %ld\n", clk_strs[i],
3c2c758a1607ea Yang Xiwen 2024-02-16 828 PTR_ERR(priv->clks[i]));
542ae60af24f02 Dongpo Li 2016-07-15 829 ret = -ENODEV;
542ae60af24f02 Dongpo Li 2016-07-15 830 goto out_free_netdev;
542ae60af24f02 Dongpo Li 2016-07-15 831 }
542ae60af24f02 Dongpo Li 2016-07-15 832 }
542ae60af24f02 Dongpo Li 2016-07-15 833
542ae60af24f02 Dongpo Li 2016-07-15 834 priv->mac_rst = devm_reset_control_get(dev, "mac");
542ae60af24f02 Dongpo Li 2016-07-15 835 if (IS_ERR(priv->mac_rst)) {
542ae60af24f02 Dongpo Li 2016-07-15 836 ret = PTR_ERR(priv->mac_rst);
3c2c758a1607ea Yang Xiwen 2024-02-16 837 goto out_free_netdev;
542ae60af24f02 Dongpo Li 2016-07-15 838 }
542ae60af24f02 Dongpo Li 2016-07-15 839 hisi_femac_core_reset(priv);
542ae60af24f02 Dongpo Li 2016-07-15 840
542ae60af24f02 Dongpo Li 2016-07-15 841 priv->phy_rst = devm_reset_control_get(dev, "phy");
542ae60af24f02 Dongpo Li 2016-07-15 842 if (IS_ERR(priv->phy_rst)) {
542ae60af24f02 Dongpo Li 2016-07-15 843 priv->phy_rst = NULL;
542ae60af24f02 Dongpo Li 2016-07-15 844 } else {
542ae60af24f02 Dongpo Li 2016-07-15 845 ret = of_property_read_u32_array(node,
542ae60af24f02 Dongpo Li 2016-07-15 846 PHY_RESET_DELAYS_PROPERTY,
542ae60af24f02 Dongpo Li 2016-07-15 847 priv->phy_reset_delays,
542ae60af24f02 Dongpo Li 2016-07-15 848 DELAYS_NUM);
542ae60af24f02 Dongpo Li 2016-07-15 849 if (ret)
3c2c758a1607ea Yang Xiwen 2024-02-16 850 goto out_free_netdev;
542ae60af24f02 Dongpo Li 2016-07-15 851 hisi_femac_phy_reset(priv);
542ae60af24f02 Dongpo Li 2016-07-15 852 }
542ae60af24f02 Dongpo Li 2016-07-15 853
3c2c758a1607ea Yang Xiwen 2024-02-16 854 // Register the optional MDIO bus
3c2c758a1607ea Yang Xiwen 2024-02-16 855 for_each_available_child_of_node(node, mdio_np) {
3c2c758a1607ea Yang Xiwen 2024-02-16 856 if (of_node_name_prefix(mdio_np, "mdio")) {
3c2c758a1607ea Yang Xiwen 2024-02-16 857 priv->mdio_pdev = of_platform_device_create(mdio_np, NULL, dev);
3c2c758a1607ea Yang Xiwen 2024-02-16 858 of_node_put(mdio_np);
3c2c758a1607ea Yang Xiwen 2024-02-16 859 if (!priv->mdio_pdev) {
3c2c758a1607ea Yang Xiwen 2024-02-16 860 dev_err(dev, "failed to register MDIO bus device\n");
ret = -ENOMEM;
3c2c758a1607ea Yang Xiwen 2024-02-16 861 goto out_free_netdev;
3c2c758a1607ea Yang Xiwen 2024-02-16 862 }
3c2c758a1607ea Yang Xiwen 2024-02-16 863 mdio_registered = true;
3c2c758a1607ea Yang Xiwen 2024-02-16 864 break;
3c2c758a1607ea Yang Xiwen 2024-02-16 865 }
3c2c758a1607ea Yang Xiwen 2024-02-16 866 }
3c2c758a1607ea Yang Xiwen 2024-02-16 867
3c2c758a1607ea Yang Xiwen 2024-02-16 868 if (!mdio_registered)
3c2c758a1607ea Yang Xiwen 2024-02-16 869 dev_warn(dev, "MDIO subnode notfound. This is usually a bug.\n");
3c2c758a1607ea Yang Xiwen 2024-02-16 870
542ae60af24f02 Dongpo Li 2016-07-15 871 phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link);
542ae60af24f02 Dongpo Li 2016-07-15 872 if (!phy) {
542ae60af24f02 Dongpo Li 2016-07-15 873 dev_err(dev, "connect to PHY failed!\n");
542ae60af24f02 Dongpo Li 2016-07-15 874 ret = -ENODEV;
3c2c758a1607ea Yang Xiwen 2024-02-16 875 goto out_unregister_mdio_bus;
542ae60af24f02 Dongpo Li 2016-07-15 876 }
542ae60af24f02 Dongpo Li 2016-07-15 877
542ae60af24f02 Dongpo Li 2016-07-15 878 phy_attached_print(phy, "phy_id=0x%.8lx, phy_mode=%s\n",
542ae60af24f02 Dongpo Li 2016-07-15 879 (unsigned long)phy->phy_id,
542ae60af24f02 Dongpo Li 2016-07-15 880 phy_modes(phy->interface));
542ae60af24f02 Dongpo Li 2016-07-15 881
9ca01b25dffffe Jakub Kicinski 2021-10-06 882 ret = of_get_ethdev_address(node, ndev);
83216e3988cd19 Michael Walle 2021-04-12 883 if (ret) {
542ae60af24f02 Dongpo Li 2016-07-15 884 eth_hw_addr_random(ndev);
542ae60af24f02 Dongpo Li 2016-07-15 885 dev_warn(dev, "using random MAC address %pM\n",
542ae60af24f02 Dongpo Li 2016-07-15 886 ndev->dev_addr);
542ae60af24f02 Dongpo Li 2016-07-15 887 }
542ae60af24f02 Dongpo Li 2016-07-15 888
542ae60af24f02 Dongpo Li 2016-07-15 889 ndev->watchdog_timeo = 6 * HZ;
542ae60af24f02 Dongpo Li 2016-07-15 890 ndev->priv_flags |= IFF_UNICAST_FLT;
542ae60af24f02 Dongpo Li 2016-07-15 891 ndev->netdev_ops = &hisi_femac_netdev_ops;
542ae60af24f02 Dongpo Li 2016-07-15 892 ndev->ethtool_ops = &hisi_femac_ethtools_ops;
b707b89f7be361 Jakub Kicinski 2022-05-06 893 netif_napi_add_weight(ndev, &priv->napi, hisi_femac_poll,
b707b89f7be361 Jakub Kicinski 2022-05-06 894 FEMAC_POLL_WEIGHT);
542ae60af24f02 Dongpo Li 2016-07-15 895
542ae60af24f02 Dongpo Li 2016-07-15 896 hisi_femac_port_init(priv);
542ae60af24f02 Dongpo Li 2016-07-15 897
542ae60af24f02 Dongpo Li 2016-07-15 898 ret = hisi_femac_init_tx_and_rx_queues(priv);
542ae60af24f02 Dongpo Li 2016-07-15 899 if (ret)
542ae60af24f02 Dongpo Li 2016-07-15 900 goto out_disconnect_phy;
542ae60af24f02 Dongpo Li 2016-07-15 901
542ae60af24f02 Dongpo Li 2016-07-15 902 ndev->irq = platform_get_irq(pdev, 0);
ae1d60c41e581b Ruan Jinjie 2023-07-31 903 if (ndev->irq < 0) {
ae1d60c41e581b Ruan Jinjie 2023-07-31 904 ret = ndev->irq;
542ae60af24f02 Dongpo Li 2016-07-15 905 goto out_disconnect_phy;
542ae60af24f02 Dongpo Li 2016-07-15 906 }
542ae60af24f02 Dongpo Li 2016-07-15 907
542ae60af24f02 Dongpo Li 2016-07-15 908 ret = devm_request_irq(dev, ndev->irq, hisi_femac_interrupt,
542ae60af24f02 Dongpo Li 2016-07-15 909 IRQF_SHARED, pdev->name, ndev);
542ae60af24f02 Dongpo Li 2016-07-15 910 if (ret) {
542ae60af24f02 Dongpo Li 2016-07-15 911 dev_err(dev, "devm_request_irq %d failed!\n", ndev->irq);
542ae60af24f02 Dongpo Li 2016-07-15 912 goto out_disconnect_phy;
542ae60af24f02 Dongpo Li 2016-07-15 913 }
542ae60af24f02 Dongpo Li 2016-07-15 914
542ae60af24f02 Dongpo Li 2016-07-15 915 ret = register_netdev(ndev);
542ae60af24f02 Dongpo Li 2016-07-15 916 if (ret) {
542ae60af24f02 Dongpo Li 2016-07-15 917 dev_err(dev, "register_netdev failed!\n");
542ae60af24f02 Dongpo Li 2016-07-15 918 goto out_disconnect_phy;
542ae60af24f02 Dongpo Li 2016-07-15 919 }
542ae60af24f02 Dongpo Li 2016-07-15 920
542ae60af24f02 Dongpo Li 2016-07-15 921 return ret;
542ae60af24f02 Dongpo Li 2016-07-15 922
542ae60af24f02 Dongpo Li 2016-07-15 923 out_disconnect_phy:
542ae60af24f02 Dongpo Li 2016-07-15 924 netif_napi_del(&priv->napi);
542ae60af24f02 Dongpo Li 2016-07-15 925 phy_disconnect(phy);
3c2c758a1607ea Yang Xiwen 2024-02-16 926 out_unregister_mdio_bus:
3c2c758a1607ea Yang Xiwen 2024-02-16 927 platform_device_unregister(priv->mdio_pdev);
542ae60af24f02 Dongpo Li 2016-07-15 928 out_free_netdev:
542ae60af24f02 Dongpo Li 2016-07-15 929 free_netdev(ndev);
542ae60af24f02 Dongpo Li 2016-07-15 930
542ae60af24f02 Dongpo Li 2016-07-15 @931 return ret;
542ae60af24f02 Dongpo Li 2016-07-15 932 }
diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c index 2406263c9dd3..d72160efff9a 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_BUS, + 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_BUS]); /* 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_BUS]); } 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_BUS] = "bus", + [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",}, {}, };