| Message ID | 20240828032343.1218749-9-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Simplified with scoped function | expand |
On Wed, 28 Aug 2024 11:23:38 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote: > Use the dev_err_probe() helper to simplify code. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Ah. I should have read next patch. Sorry! Might be worth breaking from rule of aligning parameters after brackets to keep the longest line lengths down. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > v2: > - Split into 2 patches. > --- > drivers/net/mdio/mdio-mux-mmioreg.c | 45 ++++++++++++----------------- > 1 file changed, 19 insertions(+), 26 deletions(-) > > diff --git a/drivers/net/mdio/mdio-mux-mmioreg.c b/drivers/net/mdio/mdio-mux-mmioreg.c > index 4d87e61fec7b..08c484ccdcbe 100644 > --- a/drivers/net/mdio/mdio-mux-mmioreg.c > +++ b/drivers/net/mdio/mdio-mux-mmioreg.c > @@ -109,30 +109,26 @@ 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; > + 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 +138,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", I'd align these super long ones as. "mdio-mux child node %pOF has a 'reg' value with unmasked bits\n", It is ugly but then so are > 100 char lines. > + np2); > } > > ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
On 2024/8/28 20:48, Jonathan Cameron wrote: > On Wed, 28 Aug 2024 11:23:38 +0800 > Jinjie Ruan <ruanjinjie@huawei.com> wrote: > >> Use the dev_err_probe() helper to simplify code. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > Ah. I should have read next patch. Sorry! > > Might be worth breaking from rule of aligning parameters > after brackets to keep the longest line lengths down. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> --- >> v2: >> - Split into 2 patches. >> --- >> drivers/net/mdio/mdio-mux-mmioreg.c | 45 ++++++++++++----------------- >> 1 file changed, 19 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/net/mdio/mdio-mux-mmioreg.c b/drivers/net/mdio/mdio-mux-mmioreg.c >> index 4d87e61fec7b..08c484ccdcbe 100644 >> --- a/drivers/net/mdio/mdio-mux-mmioreg.c >> +++ b/drivers/net/mdio/mdio-mux-mmioreg.c >> @@ -109,30 +109,26 @@ 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; >> + 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 +138,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", > I'd align these super long ones as. > "mdio-mux child node %pOF has a 'reg' value with unmasked bits\n", > It is ugly but then so are > 100 char lines. It seems that this kind string > 100 char is fine for patch check script. >> + np2); >> } >> >> ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node, >
> >> + 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", > > I'd align these super long ones as. > > "mdio-mux child node %pOF has a 'reg' value with unmasked bits\n", > > It is ugly but then so are > 100 char lines. > > It seems that this kind string > 100 char is fine for patch check script. Strings like this can ignore the 80 char limit because developers are going to grep for it when it shows up in their kernel log. If it gets broken in odd places, grep will not find it. I would also say the indentation is correct as is, level with &pdev. Andrew
diff --git a/drivers/net/mdio/mdio-mux-mmioreg.c b/drivers/net/mdio/mdio-mux-mmioreg.c index 4d87e61fec7b..08c484ccdcbe 100644 --- a/drivers/net/mdio/mdio-mux-mmioreg.c +++ b/drivers/net/mdio/mdio-mux-mmioreg.c @@ -109,30 +109,26 @@ 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; + 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 +138,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,
Use the dev_err_probe() helper to simplify code. Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- v2: - Split into 2 patches. --- drivers/net/mdio/mdio-mux-mmioreg.c | 45 ++++++++++++----------------- 1 file changed, 19 insertions(+), 26 deletions(-)