| Message ID | 20240830031325.2406672-6-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: Simplified with scoped function | expand |
On 8/29/2024 8:13 PM, Jinjie Ruan wrote: > Use the dev_err_probe() helper to simplify code. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > v4: > - Remove the extra parentheses. > v3: > - Add Reviewed-by. > v2: > - Split into 2 patches. > --- > drivers/net/mdio/mdio-mux-mmioreg.c | 48 ++++++++++++----------------- > 1 file changed, 20 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/mdio/mdio-mux-mmioreg.c b/drivers/net/mdio/mdio-mux-mmioreg.c > index 4d87e61fec7b..b70e6d1ad429 100644 > --- a/drivers/net/mdio/mdio-mux-mmioreg.c > +++ b/drivers/net/mdio/mdio-mux-mmioreg.c > @@ -109,30 +109,25 @@ static int mdio_mux_mmioreg_probe(struct platform_device *pdev) > return -ENOMEM; > > ret = of_address_to_resource(np, 0, &res); > - if (ret) { > - dev_err(&pdev->dev, "could not obtain memory map for node %pOF\n", > - np); > - return ret; > - } > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "could not obtain memory map for node %pOF\n", np); Besides that one, which I don't think is even a candidate for resource deferral in the first place given the OF platform implementation, it does not seem to help that much to switch to dev_err_probe() other than just combining the error message and return code in a single statement. So it's fewer lines of codes, but it is not exactly what dev_err_probe() was originally intended for IMHO. Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > @@ -109,30 +109,25 @@ static int mdio_mux_mmioreg_probe(struct platform_device *pdev) > > return -ENOMEM; > > ret = of_address_to_resource(np, 0, &res); > > - if (ret) { > > - dev_err(&pdev->dev, "could not obtain memory map for node %pOF\n", > > - np); > > - return ret; > > - } > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, > > + "could not obtain memory map for node %pOF\n", np); > > Besides that one, which I don't think is even a candidate for resource > deferral in the first place given the OF platform implementation, it does > not seem to help that much to switch to dev_err_probe() other than just > combining the error message and return code in a single statement. So it's > fewer lines of codes, but it is not exactly what dev_err_probe() was > originally intended for IMHO. Agreed. Rather than abuse dev_err_probe(), maybe a dev_err_error() would be added with the same prototype, does the same formatting, and skips all the PROBE_DEFFER logic. The problem would be, it would encourage more of this churn. Andrew
diff --git a/drivers/net/mdio/mdio-mux-mmioreg.c b/drivers/net/mdio/mdio-mux-mmioreg.c index 4d87e61fec7b..b70e6d1ad429 100644 --- a/drivers/net/mdio/mdio-mux-mmioreg.c +++ b/drivers/net/mdio/mdio-mux-mmioreg.c @@ -109,30 +109,25 @@ static int mdio_mux_mmioreg_probe(struct platform_device *pdev) return -ENOMEM; ret = of_address_to_resource(np, 0, &res); - if (ret) { - dev_err(&pdev->dev, "could not obtain memory map for node %pOF\n", - np); - return ret; - } + if (ret) + return dev_err_probe(&pdev->dev, ret, + "could not obtain memory map for node %pOF\n", np); s->phys = res.start; s->iosize = resource_size(&res); if (s->iosize != sizeof(uint8_t) && s->iosize != sizeof(uint16_t) && - s->iosize != sizeof(uint32_t)) { - dev_err(&pdev->dev, "only 8/16/32-bit registers are supported\n"); - return -EINVAL; - } + s->iosize != sizeof(uint32_t)) + return dev_err_probe(&pdev->dev, -EINVAL, + "only 8/16/32-bit registers are supported\n"); iprop = of_get_property(np, "mux-mask", &len); - if (!iprop || len != sizeof(uint32_t)) { - dev_err(&pdev->dev, "missing or invalid mux-mask property\n"); - return -ENODEV; - } - if (be32_to_cpup(iprop) >= BIT(s->iosize * 8)) { - dev_err(&pdev->dev, "only 8/16/32-bit registers are supported\n"); - return -EINVAL; - } + if (!iprop || len != sizeof(uint32_t)) + return dev_err_probe(&pdev->dev, -ENODEV, + "missing or invalid mux-mask property\n"); + if (be32_to_cpup(iprop) >= BIT(s->iosize * 8)) + return dev_err_probe(&pdev->dev, -EINVAL, + "only 8/16/32-bit registers are supported\n"); s->mask = be32_to_cpup(iprop); /* @@ -142,17 +137,14 @@ static int mdio_mux_mmioreg_probe(struct platform_device *pdev) for_each_available_child_of_node_scoped(np, np2) { u64 reg; - if (of_property_read_reg(np2, 0, ®, NULL)) { - dev_err(&pdev->dev, "mdio-mux child node %pOF is " - "missing a 'reg' property\n", np2); - return -ENODEV; - } - if ((u32)reg & ~s->mask) { - dev_err(&pdev->dev, "mdio-mux child node %pOF has " - "a 'reg' value with unmasked bits\n", - np2); - return -ENODEV; - } + if (of_property_read_reg(np2, 0, ®, NULL)) + return dev_err_probe(&pdev->dev, -ENODEV, + "mdio-mux child node %pOF is missing a 'reg' property\n", + np2); + if ((u32)reg & ~s->mask) + return dev_err_probe(&pdev->dev, -ENODEV, + "mdio-mux child node %pOF has a 'reg' value with unmasked bits\n", + np2); } ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,