| Message ID | 20211117160718.122929-1-jonas.gorski@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Revert "net: ethernet: bgmac: Use devm_platform_ioremap_resource_byname" | expand |
On Wed, Nov 17, 2021 at 05:07:18PM +0100, Jonas Gorski wrote: > This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. > > Since idm_base and nicpm_base are still optional resources not present > on all platforms, this breaks the driver for everything except Northstar > 2 (which has both). > > The same change was already reverted once with 755f5738ff98 ("net: > broadcom: fix a mistake about ioremap resource"). > > So let's do it again. Hi Jonas It is worth adding a comment in the code about them being optional. It seems like bot handlers are dumber than the bots they use, but they might read a comment and not make the same mistake a 3rd time. Andrew
On 11/17/21 8:07 AM, Jonas Gorski wrote: > This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. > > Since idm_base and nicpm_base are still optional resources not present > on all platforms, this breaks the driver for everything except Northstar > 2 (which has both). > > The same change was already reverted once with 755f5738ff98 ("net: > broadcom: fix a mistake about ioremap resource"). > > So let's do it again. > > Fixes: 3710e80952cf ("net: ethernet: bgmac: Use devm_platform_ioremap_resource_byname") > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com> Thanks!
Hi Andrew, On Wed, 17 Nov 2021 at 17:42, Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, Nov 17, 2021 at 05:07:18PM +0100, Jonas Gorski wrote: > > This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. > > > > Since idm_base and nicpm_base are still optional resources not present > > on all platforms, this breaks the driver for everything except Northstar > > 2 (which has both). > > > > The same change was already reverted once with 755f5738ff98 ("net: > > broadcom: fix a mistake about ioremap resource"). > > > > So let's do it again. > > Hi Jonas > > It is worth adding a comment in the code about them being optional. It > seems like bot handlers are dumber than the bots they use, but they > might read a comment and not make the same mistake a 3rd time. Sounds reasonable, will spin a v2 with a comment added. Regards Jonas
On 11/18/2021 1:14 AM, Jonas Gorski wrote: > Hi Andrew, > > On Wed, 17 Nov 2021 at 17:42, Andrew Lunn <andrew@lunn.ch> wrote: >> >> On Wed, Nov 17, 2021 at 05:07:18PM +0100, Jonas Gorski wrote: >>> This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. >>> >>> Since idm_base and nicpm_base are still optional resources not present >>> on all platforms, this breaks the driver for everything except Northstar >>> 2 (which has both). >>> >>> The same change was already reverted once with 755f5738ff98 ("net: >>> broadcom: fix a mistake about ioremap resource"). >>> >>> So let's do it again. >> >> Hi Jonas >> >> It is worth adding a comment in the code about them being optional. It >> seems like bot handlers are dumber than the bots they use, but they >> might read a comment and not make the same mistake a 3rd time. > > Sounds reasonable, will spin a v2 with a comment added. I just hit that problem as well refreshing my Northstar Plus to boot net-next, are you going to submit this patch in the next few days? Thanks!
On Thu, 2 Dec 2021 at 04:55, Florian Fainelli <f.fainelli@gmail.com> wrote: > On 11/18/2021 1:14 AM, Jonas Gorski wrote: > > Hi Andrew, > > > > On Wed, 17 Nov 2021 at 17:42, Andrew Lunn <andrew@lunn.ch> wrote: > >> > >> On Wed, Nov 17, 2021 at 05:07:18PM +0100, Jonas Gorski wrote: > >>> This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. > >>> > >>> Since idm_base and nicpm_base are still optional resources not present > >>> on all platforms, this breaks the driver for everything except Northstar > >>> 2 (which has both). > >>> > >>> The same change was already reverted once with 755f5738ff98 ("net: > >>> broadcom: fix a mistake about ioremap resource"). > >>> > >>> So let's do it again. > >> > >> Hi Jonas > >> > >> It is worth adding a comment in the code about them being optional. It > >> seems like bot handlers are dumber than the bots they use, but they > >> might read a comment and not make the same mistake a 3rd time. > > > > Sounds reasonable, will spin a v2 with a comment added. > > I just hit that problem as well refreshing my Northstar Plus to boot > net-next, are you going to submit this patch in the next few days? Thanks! Ah sorry, got sidetracked by a nasty security issue in our software. Will do so later today. Regards Jonas
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c index c6412c523637..d8b5e8dca7d0 100644 --- a/drivers/net/ethernet/broadcom/bgmac-platform.c +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c @@ -172,6 +172,7 @@ static int bgmac_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct bgmac *bgmac; + struct resource *regs; int ret; bgmac = bgmac_alloc(&pdev->dev); @@ -208,15 +209,21 @@ static int bgmac_probe(struct platform_device *pdev) if (IS_ERR(bgmac->plat.base)) return PTR_ERR(bgmac->plat.base); - bgmac->plat.idm_base = devm_platform_ioremap_resource_byname(pdev, "idm_base"); - if (IS_ERR(bgmac->plat.idm_base)) - return PTR_ERR(bgmac->plat.idm_base); - else + regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "idm_base"); + if (regs) { + bgmac->plat.idm_base = devm_ioremap_resource(&pdev->dev, regs); + if (IS_ERR(bgmac->plat.idm_base)) + return PTR_ERR(bgmac->plat.idm_base); bgmac->feature_flags &= ~BGMAC_FEAT_IDM_MASK; + } - bgmac->plat.nicpm_base = devm_platform_ioremap_resource_byname(pdev, "nicpm_base"); - if (IS_ERR(bgmac->plat.nicpm_base)) - return PTR_ERR(bgmac->plat.nicpm_base); + regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base"); + if (regs) { + bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev, + regs); + if (IS_ERR(bgmac->plat.nicpm_base)) + return PTR_ERR(bgmac->plat.nicpm_base); + } bgmac->read = platform_bgmac_read; bgmac->write = platform_bgmac_write;
This reverts commit 3710e80952cf2dc48257ac9f145b117b5f74e0a5. Since idm_base and nicpm_base are still optional resources not present on all platforms, this breaks the driver for everything except Northstar 2 (which has both). The same change was already reverted once with 755f5738ff98 ("net: broadcom: fix a mistake about ioremap resource"). So let's do it again. Fixes: 3710e80952cf ("net: ethernet: bgmac: Use devm_platform_ioremap_resource_byname") Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- .../net/ethernet/broadcom/bgmac-platform.c | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)