On Sunday 13 March 2016 00:34:02 Sergei Shtylyov wrote: > IS_ERR_OR_NULL() is open coded in of_mdiobus_register_phy(), so just call > it directly... > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx> > Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx> > > --- > Changes in version 2: > - removed the of_mdiobus_register_device() hunk; > - added the "Reviewed-by:" tag. > > drivers/of/of_mdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: net-next/drivers/of/of_mdio.c > =================================================================== > --- net-next.orig/drivers/of/of_mdio.c > +++ net-next/drivers/of/of_mdio.c > @@ -56,7 +56,7 @@ static int of_mdiobus_register_phy(struc > phy = phy_device_create(mdio, addr, phy_id, 0, NULL); > else > phy = get_phy_device(mdio, addr, is_c45); > - if (!phy || IS_ERR(phy)) > + if (IS_ERR_OR_NULL(phy)) > return 1; > > rc = irq_of_parse_and_map(child, 0); > > IS_ERR_OR_NULL() usually indicates that the code is wrong, or that an API has been misdesigned. Can you clarify in the changelog which one it is, or (better) change it so that 'phy' in this function is either NULL or IS_ERR() in case of an error but not both? I think moving the 'if (problem) return 1' into the two if/else is a correct transformation that also makes it very clear what is going on here. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
