On Mon, May 14, 2018 at 04:47:26PM +0300, Dan Carpenter wrote: > On Mon, May 07, 2018 at 11:20:29AM +1000, James Kelly wrote: > > +static int clk_wzrd_probe(struct platform_device *pdev) > > +{ > > + int ret; > > + struct device *dev = &pdev->dev; > > + > > + ret = clk_wzrd_get_device_tree_data(dev); > > + if (ret) > > + return ret; > > + > > + ret = clk_wzrd_regmap_alloc(dev); > > + if (ret) > > + return ret; > > + > > + ret = clk_wzrd_register_ccf(dev); > > + if (ret) > > + return ret; > > + > > + return 0; > > The error handling is a terrible layer violation now... Every > allocation function should have a matching free function. But now > instead of that if clk_wzrd_register_ccf() fails then it starts cleaning > up the clk_wzrd->axi_clk that was allocated in > clk_wzrd_get_device_tree_data(). > Oh... Duh to me. It's also buggy because clk_wzrd_regmap_alloc() can fail and then ->axi_clk isn't disabled. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel