Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 17 Oct 2022 21:48:40 -0500
From:   "han.xu" <han.xu@nxp.com>
To:     =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
Cc:     Ansuel Smith <ansuelsmth@gmail.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jens Axboe <axboe@kernel.dk>,
        =?utf-8?B?77+9ZWNraQ==?= <rafal@milecki.pl>,
        Manivannan Sadhasivam <mani@kernel.org>,
        linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/3] mtd: core: introduce of support for dynamic
 partitions
Message-ID: <20221018024840.z2qzt4kac2jilbq7@umbrella>
References: <20220606151417.19227-1-ansuelsmth@gmail.com>
 <20220606151417.19227-4-ansuelsmth@gmail.com>
 <20221017195820.ve6c3zj2civkswm6@umbrella>
 <d98dd483-70b9-4bd1-0463-9e3343576955@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <d98dd483-70b9-4bd1-0463-9e3343576955@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ak04ZUlRSmwwWCsxVVE4Y2RTdEgxL1d4Nko4V3pOQnJGZk4zelhIck5Cdm1E?=
 =?utf-8?B?cURCN01vTFVjRjR1YklOSkVVcWtDY0hhdVluTk9aSlhEbUN4Rm5aQ0poZGZl?=
 =?utf-8?B?RnB4Qmd0WXNENXN5YXR2eE1EYlV5Q0dOdG0yQnlOc3JTRTBlelUrTGVjZ1hU?=
 =?utf-8?B?UC94YVN3VExYbDg2a2NmZURIek5OSGJmSm9DNXczdE1NdmVVemRjWWdRQndP?=
 =?utf-8?B?cVF5aWxsQ0JESlNmb1NCWXBXbkt2OWpGL0VoWDA3Q0VUL0RBQVNpWVFwTG5o?=
 =?utf-8?B?SXNzbjBJQlA5OThTa3FxRW5DeVdPYjJpTVRSWWpId0s3VE1zUm9Va0tSTlZ1?=
 =?utf-8?B?cFgrQ01KUmUxa2pZRmVORk00TFM1bUhnM0s1V1BXb0lpT1Bmd3JuSlJQL2Fq?=
 =?utf-8?B?NHJsT0tHOUgzaFpmVjNYeFhldFFpcW5ZcDdMR0JNOUNOUzBUNy95VUFRSUdD?=
 =?utf-8?B?b1hkbzY4M2xPVGVlMmlCbDZyWnJMakhtRVlBZzk3bEZ0ZDdzb1l1SjNkN1NF?=
 =?utf-8?B?ZmkxTDh4K25Zd0RnaUlIK3diN3I3bElHOFM4RlErUHIvWnE2Mk80ZWVxVWtl?=
 =?utf-8?B?ZWVFWTFHRkdjclhBdURQWVY2NWh0ZWpWVzNZdEVtZTkzdmx5eFhlQU5wSnVO?=
 =?utf-8?B?OWovdDJCTE4raUtpSWUzd3owZkJaeTIrd0wxTmlGQ1dYdUZENWNqbzYxUFNL?=
 =?utf-8?B?MGNNVEdmS2ZTOUNmVkFWalBNU3MvY3VUOHdrS2FRRlZqQldBYmZlSHhIMDB2?=
 =?utf-8?B?bU1tL0J0bWg3d3BONnJDak44Q01nMWpGK24rdllNM3hpMW1kdlFVWkExNXNY?=
 =?utf-8?B?THJJK0x0SjUvajBoRUpxL3lkRklzYUhZLy9WSFEweFBENkZ1NVJWRGtzYlpJ?=
 =?utf-8?B?aFRiR25Xakw1b1UwdmVPR294dlZreUNlajBzcGRqeERzbS85QjdrSUsxRlRm?=
 =?utf-8?B?WGRzNEdoOG1QUy9YUWtFWW1ma0hLT0Evb0poOXBEVGhDTWl2SElPZ0lNb2dZ?=
 =?utf-8?B?cllBeVZPYWxuZTdPcUQ3YklWN1RjYi9Tb0pGMTJZMW04MDE0T0ZaU3U3ZkJV?=
 =?utf-8?B?OVJSNFVZdnVVZTExRWdQdmZTZ3ZNMGxacndLVjBvcnI5OFZFV2VLWlNEaEJ6?=
 =?utf-8?B?a1VRYU4zelVHQi9XVEd4Rkk4Wk41SmxRODNiS1NyelUrdTd4eHdYbmJ5MzZ4?=
 =?utf-8?B?akdQR3paTTIvaGNGelliWVhQZzJHVFlrYzRnczV2emhZQklpVWhNSW5HbC9M?=
 =?utf-8?B?VkJBeTVZTUdvSnVJaXhhVWQ4SjlZMUNzMllXTXowMnBlR0xSUjN2bzMzbGJ2?=
 =?utf-8?B?QmFDeHFCRzQrak1ma0dpZDZQeEY5ZDB5eTZmV3lueXRwc2JwcHNvK2duTnY1?=
 =?utf-8?B?RmNvR2FaK0I0U2ZzZW11aTZzYVM2Y1daeTg0eVZWUjlNWXJQZGQvMGFzdzlv?=
 =?utf-8?B?ZWg4ZHVaaEt2R2tOaWtvUmxseWFJWWpjZ280VjJEVEsxZ0RXbWROQWxTbTBW?=
 =?utf-8?B?WnN1Q3g1eTRvQ3hqODM5ZUhrOFVhaFdQSW1RZGlsd2txZHFNaDFwZjIxOThk?=
 =?utf-8?B?elFUM3k3Qnh0dXdnd3lwN2V3Qm1DVmhoMStWYklDWjRIU3BkVEVnWENWZ3h1?=
 =?utf-8?B?N0premNjNlptemNiWlBROEtlSDhJZFE1cGFtQVVYQ0lyNmgxQitTMzJ5dS9P?=
 =?utf-8?B?dVdkOUMyc1hodDNDZ1lkdDNWQVpkQTZKOWoxRGNRdEx1QVo2RG5GbkpvVUMw?=
 =?utf-8?B?eEFzR291MU9IamYzeXA4R0hTREV6YXE4OHk0T0c4dVlTQjk4YUxCZ2s4d2xE?=
 =?utf-8?B?dDlpYW1XSlpsb2ZWamg2QUFsdm9qWHZDVnFET3JoTjllNnlpTlhzbnJKOXB5?=
 =?utf-8?B?RE5PMHp5WjZSWlFYZU9QV3owZFhZWk0xK2xPTmxlbDdiMkhpOUZqZVAzOSs3?=
 =?utf-8?B?b2pIUDhSZ2drbHZQVmlPY0FWTTA0YWs0VHE1VUo5SWJ1YmtJYjN6WjkvSEp4?=
 =?utf-8?B?Vng2dlZOd1p0azMzdytNeXBHNXlNOEJQVXVNcjJMYk5STVo2QmovL2tQTldn?=
 =?utf-8?B?R3UzMFl1MEcwQWcwcHRzVFYvbUE0NEg1T29oYnpZRGNSZnZ0bFc4R0tNOVF5?=
 =?utf-8?Q?c71yPftV+RWRyApFk1JQ5Ikbg?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d2b78219-e3fd-4a31-eefa-08dab0b34892
X-MS-Exchange-CrossTenant-AuthSource: GV1PR04MB9071.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2022 02:48:49.1600
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Uw/gTebivUq+HPry1fjTjg+pgFntly/N3CrjJltrPE81sptJjgT6bDMwpNmlxJR0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8345
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 22/10/17 11:52PM, Rafał Miłecki wrote:
> On 17.10.2022 21:59, han.xu wrote:
> > On 22/06/06 05:14PM, Ansuel Smith wrote:
> > > We have many parser that register mtd partitions at runtime. One example
> > > is the cmdlinepart or the smem-part parser where the compatible is defined
> > > in the dts and the partitions gets detected and registered by the
> > > parser. This is problematic for the NVMEM subsystem that requires an OF node
> > > to detect NVMEM cells.
> > > 
> > > To fix this problem, introduce an additional logic that will try to
> > > assign an OF node to the MTD if declared.
> > > 
> > > On MTD addition, it will be checked if the MTD has an OF node and if
> > > not declared will check if a partition with the same label is
> > > declared in DTS. If an exact match is found, the partition dynamically
> > > allocated by the parser will have a connected OF node.
> > > 
> > > The NVMEM subsystem will detect the OF node and register any NVMEM cells
> > > declared statically in the DTS.
> > > 
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > >   drivers/mtd/mtdcore.c | 49 +++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 49 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > > index 7731796024e0..807194efb580 100644
> > > --- a/drivers/mtd/mtdcore.c
> > > +++ b/drivers/mtd/mtdcore.c
> > > @@ -546,6 +546,54 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
> > >   	return 0;
> > >   }
> > > +static void mtd_check_of_node(struct mtd_info *mtd)
> > > +{
> > > +	struct device_node *partitions, *parent_dn, *mtd_dn = NULL;
> > > +	struct mtd_info *parent;
> > > +	const char *mtd_name;
> > > +	bool found = false;
> > > +	int plen;
> > > +
> > > +	/* Check if MTD already has a device node */
> > > +	if (dev_of_node(&mtd->dev))
> > > +		return;
> > > +
> > > +	/* Check if a partitions node exist */
> > > +	parent = mtd->parent;
> > > +	parent_dn = dev_of_node(&parent->dev);
> > > +	if (!parent_dn)
> > > +		return;
> > > +
> > > +	partitions = of_get_child_by_name(parent_dn, "partitions");
> > > +	if (!partitions)
> > > +		goto exit_parent;
> > > +
> > > +	/* Search if a partition is defined with the same name */
> > > +	for_each_child_of_node(partitions, mtd_dn) {
> > > +		/* Skip partition with no label */
> > > +		mtd_name = of_get_property(mtd_dn, "label", &plen);
> > > +		if (!mtd_name)
> > > +			continue;
> > > +
> > > +		if (!strncmp(mtd->name, mtd_name, plen)) {
> > > +			found = true;
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	if (!found)
> > > +		goto exit_partitions;
> > > +
> > > +	/* Set of_node only for nvmem */
> > > +	if (of_device_is_compatible(mtd_dn, "nvmem-cells"))
> > > +		mtd_set_of_node(mtd, mtd_dn);
> > > +
> > > +exit_partitions:
> > > +	of_node_put(partitions);
> > > +exit_parent:
> > > +	of_node_put(parent_dn);
> > > +}
> > > +
> > >   /**
> > >    *	add_mtd_device - register an MTD device
> > >    *	@mtd: pointer to new MTD device info structure
> > > @@ -651,6 +699,7 @@ int add_mtd_device(struct mtd_info *mtd)
> > >   	mtd->dev.devt = MTD_DEVT(i);
> > >   	dev_set_name(&mtd->dev, "mtd%d", i);
> > >   	dev_set_drvdata(&mtd->dev, mtd);
> > > +	mtd_check_of_node(mtd);
> > >   	of_node_get(mtd_get_of_node(mtd));
> > >   	error = device_register(&mtd->dev);
> > >   	if (error)
> > 
> > NXP GPMI NAND controller with 6 cmdline partitions meets refcount underflow
> > with this patch. The of_node_put(parent_dn) doesn't work with cmdline parser and
> > causes the issue.
> 
> Can you try:
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 18aa54460d36..0b4ca0aa4132 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -562,7 +562,7 @@ static void mtd_check_of_node(struct mtd_info *mtd)
>  	if (!mtd_is_partition(mtd))
>  		return;
>  	parent = mtd->parent;
> -	parent_dn = dev_of_node(&parent->dev);
> +	parent_dn = of_node_get(dev_of_node(&parent->dev));
>  	if (!parent_dn)
>  		return;

Yes, with of_node_get() the refcount issue gone.

> 
>