[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250713071954.GC18469@wp.pl>
Date: Sun, 13 Jul 2025 09:19:54 +0200
From: Stanislaw Gruszka <stf_xl@...pl>
To: Rosen Penev <rosenp@...il.com>
Cc: linux-wireless@...r.kernel.org,
Johannes Berg <johannes@...solutions.net>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:MIPS" <linux-mips@...r.kernel.org>,
"moderated list:ARM/Mediatek SoC support" <linux-arm-kernel@...ts.infradead.org>,
"moderated list:ARM/Mediatek SoC support" <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCHv4 wireless-next 5/7] wifi: rt2x00: soc: modernize probe
On Sat, Jul 12, 2025 at 02:04:46PM -0700, Rosen Penev wrote:
> Remove a bunch of static memory management functions and simplify with
> devm.
>
> Also move allocation before ieee80211_alloc_hw to get rid of goto
> statements and clarify the error handling a bit more.
>
> Signed-off-by: Rosen Penev <rosenp@...il.com>
Acked-by: Stanislaw Gruszka <stf_xl@...pl>
> ---
> .../net/wireless/ralink/rt2x00/rt2800soc.c | 185 ++++++++----------
> 1 file changed, 79 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800soc.c b/drivers/net/wireless/ralink/rt2x00/rt2800soc.c
> index a19906c35d0a..6f148dec2469 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800soc.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800soc.c
> @@ -18,6 +18,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
>
> #include "rt2x00.h"
> @@ -130,108 +131,8 @@ static int rt2800soc_write_firmware(struct rt2x00_dev *rt2x00dev,
> return 0;
> }
>
> -static void rt2x00soc_free_reg(struct rt2x00_dev *rt2x00dev)
> -{
> - kfree(rt2x00dev->rf);
> - rt2x00dev->rf = NULL;
> -
> - kfree(rt2x00dev->eeprom);
> - rt2x00dev->eeprom = NULL;
> -
> - iounmap(rt2x00dev->csr.base);
> -}
> -
> -static int rt2x00soc_alloc_reg(struct rt2x00_dev *rt2x00dev)
> -{
> - struct platform_device *pdev = to_platform_device(rt2x00dev->dev);
> - struct resource *res;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -ENODEV;
> -
> - rt2x00dev->csr.base = ioremap(res->start, resource_size(res));
> - if (!rt2x00dev->csr.base)
> - return -ENOMEM;
> -
> - rt2x00dev->eeprom = kzalloc(rt2x00dev->ops->eeprom_size, GFP_KERNEL);
> - if (!rt2x00dev->eeprom)
> - goto exit;
> -
> - rt2x00dev->rf = kzalloc(rt2x00dev->ops->rf_size, GFP_KERNEL);
> - if (!rt2x00dev->rf)
> - goto exit;
> -
> - return 0;
> -
> -exit:
> - rt2x00_probe_err("Failed to allocate registers\n");
> - rt2x00soc_free_reg(rt2x00dev);
> -
> - return -ENOMEM;
> -}
> -
> -static int rt2x00soc_probe(struct platform_device *pdev, const struct rt2x00_ops *ops)
> -{
> - struct ieee80211_hw *hw;
> - struct rt2x00_dev *rt2x00dev;
> - int retval;
> -
> - hw = ieee80211_alloc_hw(sizeof(struct rt2x00_dev), ops->hw);
> - if (!hw) {
> - rt2x00_probe_err("Failed to allocate hardware\n");
> - return -ENOMEM;
> - }
> -
> - platform_set_drvdata(pdev, hw);
> -
> - rt2x00dev = hw->priv;
> - rt2x00dev->dev = &pdev->dev;
> - rt2x00dev->ops = ops;
> - rt2x00dev->hw = hw;
> - rt2x00dev->irq = platform_get_irq(pdev, 0);
> - rt2x00dev->name = pdev->dev.driver->name;
> -
> - rt2x00dev->clk = clk_get(&pdev->dev, NULL);
> - if (IS_ERR(rt2x00dev->clk))
> - rt2x00dev->clk = NULL;
> -
> - rt2x00_set_chip_intf(rt2x00dev, RT2X00_CHIP_INTF_SOC);
> -
> - retval = rt2x00soc_alloc_reg(rt2x00dev);
> - if (retval)
> - goto exit_free_device;
> -
> - retval = rt2x00lib_probe_dev(rt2x00dev);
> - if (retval)
> - goto exit_free_reg;
> -
> - return 0;
> -
> -exit_free_reg:
> - rt2x00soc_free_reg(rt2x00dev);
> -
> -exit_free_device:
> - ieee80211_free_hw(hw);
> -
> - return retval;
> -}
> -
> -static void rt2x00soc_remove(struct platform_device *pdev)
> -{
> - struct ieee80211_hw *hw = platform_get_drvdata(pdev);
> - struct rt2x00_dev *rt2x00dev = hw->priv;
> -
> - /*
> - * Free all allocated data.
> - */
> - rt2x00lib_remove_dev(rt2x00dev);
> - rt2x00soc_free_reg(rt2x00dev);
> - ieee80211_free_hw(hw);
> -}
> -
> #ifdef CONFIG_PM
> -static int rt2x00soc_suspend(struct platform_device *pdev, pm_message_t state)
> +static int rt2800soc_suspend(struct platform_device *pdev, pm_message_t state)
> {
> struct ieee80211_hw *hw = platform_get_drvdata(pdev);
> struct rt2x00_dev *rt2x00dev = hw->priv;
> @@ -239,7 +140,7 @@ static int rt2x00soc_suspend(struct platform_device *pdev, pm_message_t state)
> return rt2x00lib_suspend(rt2x00dev);
> }
>
> -static int rt2x00soc_resume(struct platform_device *pdev)
> +static int rt2800soc_resume(struct platform_device *pdev)
> {
> struct ieee80211_hw *hw = platform_get_drvdata(pdev);
> struct rt2x00_dev *rt2x00dev = hw->priv;
> @@ -357,7 +258,77 @@ static const struct rt2x00_ops rt2800soc_ops = {
>
> static int rt2800soc_probe(struct platform_device *pdev)
> {
> - return rt2x00soc_probe(pdev, &rt2800soc_ops);
> + const struct rt2x00_ops *ops = of_device_get_match_data(&pdev->dev);
> + struct rt2x00_dev *rt2x00dev;
> + struct ieee80211_hw *hw;
> + void __iomem *mem;
> + struct clk *clk;
> + __le16 *eeprom;
> + int retval;
> + u32 *rf;
> + int irq;
> +
> + mem = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(mem))
> + return PTR_ERR(mem);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + clk = devm_clk_get_optional(&pdev->dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + eeprom = devm_kzalloc(&pdev->dev, ops->eeprom_size, GFP_KERNEL);
> + if (!eeprom)
> + return -ENOMEM;
> +
> + rf = devm_kzalloc(&pdev->dev, ops->rf_size, GFP_KERNEL);
> + if (!rf)
> + return -ENOMEM;
> +
> + hw = ieee80211_alloc_hw(sizeof(struct rt2x00_dev), ops->hw);
> + if (!hw)
> + return dev_err_probe(&pdev->dev, -ENOMEM, "Failed to allocate hardware");
> +
> + platform_set_drvdata(pdev, hw);
> +
> + rt2x00dev = hw->priv;
> + rt2x00dev->dev = &pdev->dev;
> + rt2x00dev->ops = ops;
> + rt2x00dev->hw = hw;
> + rt2x00dev->irq = irq;
> + rt2x00dev->clk = clk;
> + rt2x00dev->eeprom = eeprom;
> + rt2x00dev->rf = rf;
> + rt2x00dev->name = pdev->dev.driver->name;
> + rt2x00dev->csr.base = mem;
> +
> + rt2x00_set_chip_intf(rt2x00dev, RT2X00_CHIP_INTF_SOC);
> +
> + retval = rt2x00lib_probe_dev(rt2x00dev);
> + if (retval)
> + goto exit_free_device;
> +
> + return 0;
> +
> +exit_free_device:
> + ieee80211_free_hw(hw);
> +
> + return retval;
> +}
> +
> +static void rt2800soc_remove(struct platform_device *pdev)
> +{
> + struct ieee80211_hw *hw = platform_get_drvdata(pdev);
> + struct rt2x00_dev *rt2x00dev = hw->priv;
> +
> + /*
> + * Free all allocated data.
> + */
> + rt2x00lib_remove_dev(rt2x00dev);
> + ieee80211_free_hw(hw);
> }
>
> static const struct of_device_id rt2880_wmac_match[] = {
> @@ -372,9 +343,11 @@ static struct platform_driver rt2800soc_driver = {
> .of_match_table = rt2880_wmac_match,
> },
> .probe = rt2800soc_probe,
> - .remove = rt2x00soc_remove,
> - .suspend = rt2x00soc_suspend,
> - .resume = rt2x00soc_resume,
> + .remove = rt2800soc_remove,
> +#ifdef CONFIG_PM
> + .suspend = rt2800soc_suspend,
> + .resume = rt2800soc_resume,
> +#endif
> };
>
> module_platform_driver(rt2800soc_driver);
> --
> 2.50.0
>
Powered by blists - more mailing lists