linux-kernel - Re: [PATCH v2] regmap: Add regmap dummy driver

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]

Date:	Sat, 4 Aug 2012 11:05:23 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>
Cc:	linux-kernel@...r.kernel.org, patches@...nsource.wolfsonmicro.com
Subject: Re: [PATCH v2] regmap: Add regmap dummy driver

On Fri, Jul 27, 2012 at 02:58:20PM +0100, Dimitris Papastamos wrote:
> Add a pseudo-driver for debugging and stress-testing the
> regmap/regcache APIs.  A standard set of tools for working

Overall this looks good, most of the stuff below is fairly small.  As a
very high level comment it'd be really helpful to split this into a
series of commits, for example adding just the dummy device then
building out the functionality.  It'd make review much easier.

> with this driver (mainly sh scripts) will be put in a repo
> at https://github.com/quantumdream/regmap-tools

Any reason not to put this in the tools directory?

> Some of these tests will require one to build with
> REGMAP_ALLOW_WRITE_DEBUGFS defined.

Can we add a write mechanism specifically for this dummy driver?

> +	/* Set when regdummy defaults have been modified.
> +	 * This is useful to know so we don't reinit the
> +	 * cache if there is no reason to do so. */
> +	unsigned int dirty:1;

Should we perhaps just reinit anyway?  It's not like this is performance
critical...

> +/* Default volatile register callback, this should
> + * normally be configured by the user via a debugfs
> + * entry */
> +static bool regdummy_volatile_reg(struct device *dev,
> +				  unsigned int reg)
> +{
> +	return false;
> +}

All these functions just seem to be implementing the default behaviour,
why are they needed?

> +		/* If we're in the region the user is trying to read */
> +		if (p >= *ppos) {
> +			/* ...but not beyond it */
> +			if (buf_pos >= count - 1 - tot_len)
> +				break;

Any potential for code reuse?  This stuff does look awfully familiar!

> +	/* Allocate the new register defaults */
> +	regdef_num_new = rdevp->regs_size_new / config->reg_stride;
> +	regdef_num_raw_new = regdef_num_new * sizeof(*regdef_new);
> +	regdef_new = kzalloc(regdef_num_raw_new, GFP_KERNEL);

Can we factor this stuff out - there's a lot of overlap with the vanilla
init?

> +static struct platform_device regdummy_device = {
> +	.name = "regdummy",
> +	.id = 0,
> +};

Set id to -1 if there's only one of them.
--
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/