Re: [PATCH v2 15/58] mtd: rawnand: tegra: retain open-handle state on removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] mtd: rawnand: tegra: cleanup helper and goto mixing in
  tegra_nand_chips_init()
--

commit 1e69b67e008f7b424489423e4d072fb7d38b3000
Author: James Hilliard <james.hilliard1@gmail.com>

mtd: rawnand: tegra: retain open-handle state on removal

This commit switches the Tegra NAND chip and controller allocations to
reference-counted lifetime management so they are freed when the final MTD
references are released, and cleans up the child device node reference in
tegra_nand_chips_init().

> diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
> index 1104d75b6cbe6..5130bee46d609 100644
> --- a/drivers/mtd/nand/raw/tegra_nand.c
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -1064,11 +1064,16 @@ static const struct nand_controller_ops tegra_nand_controller_ops = {
>  	.setup_interface = tegra_nand_setup_interface,
>  };
>  
> +static void tegra_nand_chip_release(struct nand_chip *chip)
> +{
> +	kfree(container_of(chip, struct tegra_nand_chip, chip));
> +}
> +
>  static int tegra_nand_chips_init(struct device *dev,
>  				 struct tegra_nand_controller *ctrl)
>  {
>  	struct device_node *np = dev->of_node;
> -	struct device_node *np_nand;
> +	struct device_node *np_nand __free(device_node) = NULL;
>  	int nsels, nchips = of_get_child_count(np);
>  	struct tegra_nand_chip *nand;
>  	struct mtd_info *mtd;
> @@ -1096,7 +1101,7 @@ static int tegra_nand_chips_init(struct device *dev,
>  		return ret;
>  	}
>  
> -	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
> +	nand = kzalloc_obj(*nand);
>  	if (!nand)
>  		return -ENOMEM;
>  
> @@ -1107,7 +1112,7 @@ static int tegra_nand_chips_init(struct device *dev,
>  	if (IS_ERR(nand->wp_gpio)) {
>  		ret = PTR_ERR(nand->wp_gpio);
>  		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
> -		return ret;
> +		goto free_chip;
>  	}
>  
>  	chip = &nand->chip;
> @@ -1127,20 +1132,35 @@ static int tegra_nand_chips_init(struct device *dev,
>  
>  	ret = nand_scan(chip, 1);
>  	if (ret)
> -		return ret;
> +		goto free_chip;
>  
>  	mtd_ooblayout_ecc(mtd, 0, &nand->ecc);
> +	ret = nand_set_release(chip, tegra_nand_chip_release);
> +	if (ret) {
> +		nand_cleanup(chip);
> +		goto free_chip;
> +	}
>  
>  	ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret) {
>  		dev_err(dev, "Failed to register mtd device: %d\n", ret);
> -		nand_cleanup(chip);
> +		nand_disconnect(chip);
> +		mtd_put_master(mtd);
>  		return ret;
>  	}
>  
>  	ctrl->chip = chip;
>  
>  	return 0;
> +
> +free_chip:
> +	kfree(nand);
> +	return ret;
>  }

[Severity: Low]
This isn't a bug, but tegra_nand_chips_init() mixes __free(device_node) on
np_nand with a new goto free_chip unwinding path for nand, rather than
using either scope-based cleanup or goto unwinding consistently across the
function.  Also, np_nand is declared with __free(device_node) = NULL at
the top of the function instead of being defined and initialized in a
single statement where of_get_next_child() is called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-submit-rawnand-retained-handle-lifetime-v1-v2-0-9137d08f05df@gmail.com?part=15

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/