On 06/28/2016 12:34 PM, Jon Mason wrote: > The bcma portion of the driver has been split off into a bcma specific > driver. This has been mirrored for the platform driver. The last > references to the bcma core struct have been changed into a generic > function call. These function calls are wrappers to either the original > bcma code or new platform functions that access the same areas via MMIO. > This necessitated adding function pointers for both platform and bcma to > hide which backend is being used from the generic bgmac code. > > Signed-off-by: Jon Mason <jon.ma...@broadcom.com> > ---
[snip] > +static int bgmac_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct bgmac *bgmac; > + struct resource regs; > + const u8 *mac_addr; > + int rc; > + > + bgmac = kzalloc(sizeof(*bgmac), GFP_KERNEL); You could utilize devm_kzalloc() here which simplifies the error path. > + if (!bgmac) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, bgmac); > + > + /* Set the features of the 4707 family */ > + bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST; > + bgmac->feature_flags |= BGMAC_FEAT_NO_RESET; > + bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500; > + bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4; > + bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP; > + bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP; > + > + bgmac->dev = &pdev->dev; > + bgmac->dma_dev = &pdev->dev; > + > + mac_addr = of_get_mac_address(np); > + if (mac_addr) > + ether_addr_copy(bgmac->mac_addr, mac_addr); > + else > + dev_warn(&pdev->dev, "MAC address not present in device > tree\n"); > + > + bgmac->irq = platform_get_irq(pdev, 0); > + if (bgmac->irq < 0) { > + rc = bgmac->irq; > + dev_err(&pdev->dev, "Unable to obtain IRQ\n"); > + goto err; > + } > + > + rc = of_address_to_resource(np, 0, ®s); > + if (rc < 0) { > + dev_err(&pdev->dev, "Unable to obtain base resource\n"); > + goto err; > + } Here you could fetch the resource using a traditional platform_get_resource(pdev, IORESOURCE_MEM, 0) and... > + > + bgmac->plat.base = ioremap(regs.start, resource_size(®s)); > + if (!bgmac->plat.base) { > + dev_err(&pdev->dev, "Unable to map base resource\n"); > + rc = -ENOMEM; > + goto err; > + } ... here do a devm_ioremap_resource(), which also does a request_mem_region, so this shows up nicely in /proc/iomem. > + > + rc = of_address_to_resource(np, 1, ®s); > + if (rc < 0) { > + dev_err(&pdev->dev, "Unable to obtain idm resource\n"); > + goto err1; > + } > + > + bgmac->plat.idm_base = ioremap(regs.start, resource_size(®s)); > + if (!bgmac->plat.base) { > + dev_err(&pdev->dev, "Unable to map idm resource\n"); > + rc = -ENOMEM; > + goto err1; > + } Same here. > + > + bgmac->read = platform_bgmac_read; > + bgmac->write = platform_bgmac_write; > + bgmac->idm_read = platform_bgmac_idm_read; > + bgmac->idm_write = platform_bgmac_idm_write; > + bgmac->clk_enabled = platform_bgmac_clk_enabled; > + bgmac->clk_enable = platform_bgmac_clk_enable; > + bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset; > + bgmac->get_bus_clock = platform_bgmac_get_bus_clock; > + bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32; > + > + rc = bgmac_enet_probe(bgmac); > + if (rc) > + goto err2; > + > + return 0; > + > +err2: > + iounmap(bgmac->plat.idm_base); > +err1: > + iounmap(bgmac->plat.base); > +err: > + kfree(bgmac); And with devm_* helpers none of that is needed now. -- Florian