On Sun, Jan 24, 2021 at 01:43:52PM +0200, stef...@marvell.com wrote:
> +static int mvpp2_get_sram(struct platform_device *pdev,
> + struct mvpp2 *priv)
> +{
> + struct device_node *dn = pdev->dev.of_node;
> + static bool defer_once;
> + struct resource *res;
> +
> + if (has_acpi_companion(&pdev->dev)) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + if (!res) {
> + dev_warn(&pdev->dev, "ACPI is too old, Flow control not
> supported\n");
> + return 0;
> + }
> + priv->cm3_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->cm3_base))
> + return PTR_ERR(priv->cm3_base);
> + } else {
> + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> + if (!priv->sram_pool) {
> + if (!defer_once) {
> + defer_once = true;
> + /* Try defer once */
> + return -EPROBE_DEFER;
> + }
> + dev_warn(&pdev->dev, "DT is too old, Flow control not
> supported\n");
> + return -ENOMEM;
> + }
Above, priv->sram_pool will only be non-NULL if the pool has been
initialised.
> +err_cm3:
> + if (!has_acpi_companion(&pdev->dev) && priv->cm3_base)
> + gen_pool_free(priv->sram_pool, (unsigned long)priv->cm3_base,
> + MSS_SRAM_SIZE);
So wouldn't:
if (priv->sram_pool && priv->cm3_base)
be more appropriate here?
> + if (!has_acpi_companion(&pdev->dev) && priv->cm3_base) {
> + gen_pool_free(priv->sram_pool, (unsigned long)priv->cm3_base,
> + MSS_SRAM_SIZE);
> + gen_pool_destroy(priv->sram_pool);
> + }
Same here.
Why is it correct to call gen_pool_destroy() in the remove path but not
the error path? I think you want to drop this - the pool is created and
destroyed by the SRAM driver, users of it should not be destroying it.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
