linux-kernel - Re: [PATCH 3/3] mfd: syscon: Allow syscon nodes without a "syscon" compatible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241215203344.46158-1-john.madieu.xa@bp.renesas.com>
Date: Sun, 15 Dec 2024 21:33:44 +0100
From: John Madieu <john.madieu.xa@...renesas.com>
To: robh@...nel.org
Cc: arnd@...db.de,
	heiko@...ech.de,
	krzysztof.kozlowski@...aro.org,
	lee@...nel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	liviu.dudau@....com,
	lpieralisi@...nel.org,
	pankaj.dubey@...sung.com,
	peter.griffin@...aro.org,
	sudeep.holla@....com,
	willmcvicker@...gle.com,
	biju.das.jz@...renesas.com
Subject: Re: [PATCH 3/3] mfd: syscon: Allow syscon nodes without a "syscon" compatible

Hi Rob,

On Wed, 11 Dec 2024 14:57:14 -0600 Rob Herring wrote:
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index bfb1f69fcff1..e6df2825c14d 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -171,8 +171,10 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
> 			break;
> 		}
> 
> -	if (!syscon)
> +	if (!syscon && of_device_is_compatible(np, "syscon"))
> 		syscon = of_syscon_register(np, check_res);
> +	else
> +		syscon = ERR_PTR(-EINVAL);

The current modification will make device_node_get_regmap() return -EINVAL even 
for syscons that were already found in the syscon_list, which I believe is not 
the intended behavior.

I suggest modifying it this way to maintain lookup functionality for registered 
syscons while implementing your intended changes:

static struct regmap *device_node_get_regmap(struct device_node *np,
					     bool check_res)
{
	struct syscon *entry, *syscon = NULL;
	struct regmap *regmap;

	mutex_lock(&syscon_list_lock);

	list_for_each_entry(entry, &syscon_list, list)
		if (entry->np == np) {
			syscon = entry;
			break;
		}

	if (syscon) {
		regmap = syscon->regmap;
		mutex_unlock(&syscon_list_lock);
		return regmap;
	}

	if (of_device_is_compatible(np, "syscon")) {
		syscon = of_syscon_register(np, check_res);
		mutex_unlock(&syscon_list_lock);
		if (IS_ERR(syscon))
			return ERR_CAST(syscon);
		return syscon->regmap;
	}

	mutex_unlock(&syscon_list_lock);
	return ERR_PTR(-EINVAL);
}

The above is the resulting working function I've obtained while testing with
different scenarios. This ensures that:

 1. Already registered syscons are found and returned correctly
 2. New syscons with "syscon" compatible are registered as before
 3. Nodes without "syscon" compatible return -EINVAL only if not found in the list

This has been tested with my v1 syscon work [1] and with v2, where I remove the
"syscon" compatible string for custom regmap initialization, aligning with your
goals for this series.

Let me know if I should add this series as a dependency of my v2 or how I should proceed.

Thanks,

John Madieu

[1] https://lore.kernel.org/all/20241206212559.192705-2-john.madieu.xa@bp.renesas.com