LinuxLists.cc - [PATCH] [net/ethernet] arc_emac: Add error handling in emac_rockchip

2023-11-30 03:16:21

Subject: [PATCH] [net/ethernet] arc_emac: Add error handling in emac_rockchip_probe

This patch introduces error handling for the of_match_node call within
the emac_rockchip_probe. Previously, there was no check for the return
value of of_match_node, which could result in improper behavior if the
device tree match was unsuccessful.

Although the error addressed by this patch may not occur in the current
environment, I still suggest implementing these error handling routines
if the function is not highly time-sensitive. As the environment evolves
or the code gets reused in different contexts, there's a possibility that
these errors might occur. Addressing them now can prevent potential
debugging efforts in the future, which could be quite resource-intensive.

Signed-off-by: Haoran Liu <[email protected]>
---
drivers/net/ethernet/arc/emac_rockchip.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac_rockchip.c b/drivers/net/ethernet/arc/emac_rockchip.c
index 493d6356c8ca..f6f1390b77f6 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -134,6 +134,11 @@ static int emac_rockchip_probe(struct platform_device *pdev)
}

match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
+ if (!match) {
+ dev_err(dev, "No matching device found\n");
+ return -ENODEV;
+ }
+
priv->soc_data = match->data;

priv->emac.clk = devm_clk_get(dev, "hclk");
--
2.17.1

2023-12-02 04:11:57

by Jakub Kicinski

[permalink] [raw]

Subject: Re: [PATCH] [net/ethernet] arc_emac: Add error handling in emac_rockchip_probe

On Wed, 29 Nov 2023 19:13:18 -0800 Haoran Liu wrote:
> Although the error addressed by this patch may not occur in the current
> environment, I still suggest implementing these error handling routines
> if the function is not highly time-sensitive. As the environment evolves
> or the code gets reused in different contexts, there's a possibility that
> these errors might occur. Addressing them now can prevent potential
> debugging efforts in the future, which could be quite resource-intensive.

Please skip this pointless boilerplate.

> match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
> + if (!match) {
> + dev_err(dev, "No matching device found\n");
> + return -ENODEV;

"device" and ENODEV are not right here.
--
pw-bot: cr

2023-12-06 05:36:35

by Dan Carpenter

[permalink] [raw]

Subject: Re: [PATCH] [net/ethernet] arc_emac: Add error handling in emac_rockchip_probe

Hi Haoran,

kernel test robot noticed
If you fix the issue the same patch/commit), | Reported-by: kernel | Reported-by: Dan | Closes: the following build warnings:
m/docs/git-format-patch#_base_tree_information]">https://git-scm.com/docs/git-format-patch#_base_tree_information]
thub.com/intel-lab-lkp/linux/commits/Haoran-Liu/arc_emac-Add-error-handling-in-emac_rockchip_probe/20231130-112325">https://github.com/intel-lab-lkp/linux/commits/Haoran-Liu/arc_emac-Add-error-handling-in-emac_rockchip_probe/20231130-112325
t.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git">https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
href="https://lore.kernel.org/r/20231130031318.35850-1-liuhaoran14%40163.com">https://lore.kernel.org/r/20231130031318.35850-1-liuhaoran14%40163.com
[net/ethernet] arc_emac: Add error handling in emac_rockchip_probe
(https://download.01.org/0day-ci/archive/20231206/[email protected]/config)
(GCC) 13.2.0
//download.01.org/0day-ci/archive/20231206/202312060119.eRpB9j4s-lkp@intel.com/reproduce)">https://download.01.org/0day-ci/archive/20231206/[email protected]/reproduce)
in a separate patch/commit (i.e. not just a new version of
kindly add following tags
test robot <[email protected]>
Carpenter <[email protected]>
lore.kernel.org/r/202312060119.eRpB9j4s-lkp@intel.com/">https://lore.kernel.org/r/[email protected]/
c_rockchip.c:139 emac_rockchip_probe() warn: missing unwind goto?
c_rockchip.c:249 emac_rockchip_probe() warn: 'ndev' from alloc_etherdev_mqs() not released on lines: 139.
t/arc/emac_rockchip.c
2014-09-08 93 static int emac_rockchip_probe(struct platform_device *pdev)
2014-09-08 94 {
2014-09-08 95 struct device *dev = &pdev->dev;
2014-09-08 96 struct net_device *ndev;
2014-09-08 97 struct rockchip_priv_data *priv;
2014-09-08 98 const struct of_device_id *match;
2019-11-04 99 phy_interface_t interface;
2014-09-08 100 u32 data;
2019-11-04 101 int err;
2014-09-08 102
2014-09-08 103 if (!pdev->dev.of_node)
2014-09-08 104 return -ENODEV;
2014-09-08 105
2014-09-08 106 ndev = alloc_etherdev(sizeof(struct rockchip_priv_data));
2014-09-08 107 if (!ndev)
2014-09-08 108 return -ENOMEM;
2014-09-08 109 platform_set_drvdata(pdev, ndev);
2014-09-08 110 SET_NETDEV_DEV(ndev, dev);
2014-09-08 111
2014-09-08 112 priv = netdev_priv(ndev);
2014-09-08 113 priv->emac.drv_name = DRV_NAME;
2014-09-08 114 priv->emac.set_mac_speed = emac_rockchip_set_mac_speed;
2014-09-08 115
2019-11-04 116 err = of_get_phy_mode(dev->of_node, &interface);
2019-11-04 117 if (err)
2019-11-04 118 goto out_netdev;
2014-09-08 119
2016-01-08 120 /* RK3036/RK3066/RK3188 SoCs only support RMII */
2014-09-08 121 if (interface != PHY_INTERFACE_MODE_RMII) {
2014-09-08 122 dev_err(dev, "unsupported phy interface mode %d\n", interface);
2014-09-08 123 err = -ENOTSUPP;
2014-09-08 124 goto out_netdev;
2014-09-08 125 }
2014-09-08 126
2016-03-14 127 priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
2016-03-14 128 "rockchip,grf");
2014-09-08 129 if (IS_ERR(priv->grf)) {
2016-03-14 130 dev_err(dev, "failed to retrieve global register file (%ld)\n",
2016-03-14 131 PTR_ERR(priv->grf));
2014-09-08 132 err = PTR_ERR(priv->grf);
2014-09-08 133 goto out_netdev;
2014-09-08 134 }
2014-09-08 135
2014-09-08 136 match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
2023-11-29 137 if (!match) {
2023-11-29 138 dev_err(dev, "No matching device found\n");
2023-11-29 @139 return -ENODEV;
2023-11-29 140 }
2023-11-29 141
2014-09-08 142 priv->soc_data = match->data;
2014-09-08 143
2014-09-08 144 priv->emac.clk = devm_clk_get(dev, "hclk");
2014-09-08 145 if (IS_ERR(priv->emac.clk)) {
2016-03-14 146 dev_err(dev, "failed to retrieve host clock (%ld)\n",
2016-03-14 147 PTR_ERR(priv->emac.clk));
2014-09-08 148 err = PTR_ERR(priv->emac.clk);
2014-09-08 149 goto out_netdev;
2014-09-08 150 }
2014-09-08 151
2014-09-08 152 priv->refclk = devm_clk_get(dev, "macref");
2014-09-08 153 if (IS_ERR(priv->refclk)) {
2016-03-14 154 dev_err(dev, "failed to retrieve reference clock (%ld)\n",
2016-03-14 155 PTR_ERR(priv->refclk));
2014-09-08 156 err = PTR_ERR(priv->refclk);
2014-09-08 157 goto out_netdev;
2014-09-08 158 }
2014-09-08 159
2014-09-08 160 err = clk_prepare_enable(priv->refclk);
2014-09-08 161 if (err) {
2014-09-08 162 dev_err(dev, "failed to enable reference clock (%d)\n", err);
2014-09-08 163 goto out_netdev;
2014-09-08 164 }
2014-09-08 165
2014-09-08 166 /* Optional regulator for PHY */
2014-09-08 167 priv->regulator = devm_regulator_get_optional(dev, "phy");
2014-09-08 168 if (IS_ERR(priv->regulator)) {
JAILLET 2018-03-18 169 if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
JAILLET 2018-03-18 170 err = -EPROBE_DEFER;
JAILLET 2018-03-18 171 goto out_clk_disable;
JAILLET 2018-03-18 172 }
2014-09-08 173 dev_err(dev, "no regulator found\n");
2014-09-08 174 priv->regulator = NULL;
2014-09-08 175 }
2014-09-08 176
2014-09-08 177 if (priv->regulator) {
2014-09-08 178 err = regulator_enable(priv->regulator);
2014-09-08 179 if (err) {
2014-09-08 180 dev_err(dev, "failed to enable phy-supply (%d)\n", err);
2014-09-08 181 goto out_clk_disable;
2014-09-08 182 }
2014-09-08 183 }
2014-09-08 184
2016-01-08 185 /* Set speed 100M */
2016-01-08 186 data = (1 << (priv->soc_data->grf_speed_offset + 16)) |
2016-01-08 187 (1 << priv->soc_data->grf_speed_offset);
2016-01-08 188 /* Set RMII mode */
2016-01-08 189 data |= (1 << (priv->soc_data->grf_mode_offset + 16)) |
2016-01-08 190 (0 << priv->soc_data->grf_mode_offset);
2014-09-08 191
2014-09-08 192 err = regmap_write(priv->grf, priv->soc_data->grf_offset, data);
2014-09-08 193 if (err) {
2016-03-14 194 dev_err(dev, "unable to apply initial settings to grf (%d)\n",
2016-03-14 195 err);
2014-09-08 196 goto out_regulator_disable;
2014-09-08 197 }
2014-09-08 198
2014-09-08 199 /* RMII interface needs always a rate of 50MHz */
2014-09-08 200 err = clk_set_rate(priv->refclk, 50000000);
2017-12-12 201 if (err) {
2016-03-14 202 dev_err(dev,
2016-03-14 203 "failed to change reference clock rate (%d)\n", err);
2017-12-12 204 goto out_regulator_disable;
2017-12-12 205 }
2016-01-08 206
2016-01-08 207 if (priv->soc_data->need_div_macclk) {
2016-01-08 208 priv->macclk = devm_clk_get(dev, "macclk");
2016-01-08 209 if (IS_ERR(priv->macclk)) {
2016-03-14 210 dev_err(dev, "failed to retrieve mac clock (%ld)\n",
2016-03-14 211 PTR_ERR(priv->macclk));
2016-01-08 212 err = PTR_ERR(priv->macclk);
2016-01-08 213 goto out_regulator_disable;
2016-01-08 214 }
2016-01-08 215
2016-01-08 216 err = clk_prepare_enable(priv->macclk);
2016-01-08 217 if (err) {
2016-01-08 218 dev_err(dev, "failed to enable mac clock (%d)\n", err);
2016-01-08 219 goto out_regulator_disable;
2016-01-08 220 }
2016-01-08 221
2016-01-08 222 /* RMII TX/RX needs always a rate of 25MHz */
2016-01-08 223 err = clk_set_rate(priv->macclk, 25000000);
2017-12-07 224 if (err) {
2016-03-14 225 dev_err(dev,
2016-03-14 226 "failed to change mac clock rate (%d)\n", err);
2017-12-07 227 goto out_clk_disable_macclk;
2017-12-07 228 }
2016-01-08 229 }
2016-01-08 230
2016-01-08 231 err = arc_emac_probe(ndev, interface);
2016-01-08 232 if (err) {
2016-01-08 233 dev_err(dev, "failed to probe arc emac (%d)\n", err);
2017-12-12 234 goto out_clk_disable_macclk;
2016-01-08 235 }
2016-01-08 236
2014-09-08 237 return 0;
2017-12-12 238
2017-12-07 239 out_clk_disable_macclk:
2017-12-12 240 if (priv->soc_data->need_div_macclk)
2017-12-07 241 clk_disable_unprepare(priv->macclk);
2014-09-08 242 out_regulator_disable:
2014-09-08 243 if (priv->regulator)
2014-09-08 244 regulator_disable(priv->regulator);
2014-09-08 245 out_clk_disable:
2014-09-08 246 clk_disable_unprepare(priv->refclk);
2014-09-08 247 out_netdev:
2014-09-08 248 free_netdev(ndev);
2014-09-08 @249 return err;
2014-09-08 250 }
/intel/lkp-tests/wiki">https://github.com/intel/lkp-tests/wiki