linux-kernel - Re: [PATCH v3 1/7] mfd: add syscon driver based on regmap

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: <503D4C08.8040605@wwwdotorg.org>
Date:	Tue, 28 Aug 2012 15:54:00 -0700
From:	Stephen Warren <swarren@...dotorg.org>
To:	Dong Aisheng <b29396@...escale.com>
CC:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linus.walleij@...ricsson.com, s.hauer@...gutronix.de,
	shawn.guo@...aro.org, kernel@...gutronix.de,
	grant.likely@...retlab.ca, rob.herring@...xeda.com,
	sameo@...ux.intel.com, lrg@...com,
	broonie@...nsource.wolfsonmicro.com, richard.zhao@...escale.com,
	devicetree-discuss@...ts.ozlabs.org
Subject: Re: [PATCH v3 1/7] mfd: add syscon driver based on regmap

On 08/28/2012 02:09 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@...aro.org>
> 
> Add regmap based syscon driver.
> This is usually used for access misc bits in registers which does not belong
> to a specific module, for example, IMX IOMUXC GPR and ANATOP.
> With this driver, client can use generic regmap API to access registers
> which are registered into syscon.

> diff --git a/Documentation/devicetree/bindings/mfd/syscon.txt b/Documentation/devicetree/bindings/mfd/syscon.txt

> +* System Controller Registers R/W driver

I think the binding document could use some information about what a
"syscon" actually is, especially since the compatible value is something
generic like "syscon" rather than something with a vendor-specific
prefix. How about:

A system controller node represents a register region containing a set
of miscellaneous registers. The registers are not cohesive enough to
represent as any specific type of device. The typical use-case is for
some other node's driver, or platform-specific code, to acquire a
reference to the syscon node (e.g. by phandle, node path, or search
using a specific compatible value), interrogate the node (or associated
OS driver) to determine the location of the registers, and access the
registers directly.

> +Required properties:
> +- compatible: Should contain "syscon".
> +- reg: the register range can be access from syscon
> +
> +Examples:
> +gpr: iomuxc-gpr@...e0000 {
> +	compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
> +	reg = <0x020e0000 0x38>;
> +};

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig

> +config MFD_SYSCON
> +        bool "System Controller Register R/W Based on Regmap"
> +        select REGMAP_MMIO
> +        help
> +          Select this option to enable accessing system control registers
> +	  via regmap.

I think the indentation is off there.

> +static int __devinit syscon_probe(struct platform_device *pdev)
...
> +	regcache_cache_only(syscon->regmap, false);

Isn't that the default?

Aside from that, I don't have any particular comments on this series, so
please consider it,

Acked-by: Stephen Warren <swarren@...dotorg.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/