linux-kernel - Re: [PATCH V2 1/4] mfd: Initial support for Texas Instruments AIC family of CODECs

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: <20130501154346.GO26160@sirena.org.uk>
Date:	Wed, 1 May 2013 16:43:46 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Mehar Bajwa <mehar.bajwa@...com>
Cc:	linux-kernel@...r.kernel.org, sameo@...ux.intel.com,
	liam.r.girdwood@...el.com, navada@...com
Subject: Re: [PATCH V2 1/4] mfd: Initial support for Texas Instruments AIC
 family of CODECs

On Tue, Apr 16, 2013 at 07:13:36PM +0530, Mehar Bajwa wrote:

> +/**
> + * set_aic_book: change book which we have to write/read to.
> + *
> + * @aic: Device to write/read to.
> + * @book: Book to write/read to.
> + */
> +int set_aic_book(struct aic *aic, int book)
> +{
> +	int ret = 0;
> +	u8 page_buf[] = { 0x0, 0x0 };
> +	u8 book_buf[] = { 0x7f, 0x0 };
> +
> +	ret = regmap_write(aic->regmap, page_buf[0], page_buf[1]);
> +
> +	if (ret < 0)
> +		return ret;
> +	book_buf[1] = book;
> +	ret = regmap_write(aic->regmap, book_buf[0], book_buf[1]);

This looks like just like register paging - there's support in the core
for that, if it's not adequate then just extend the framework.  Looks
like the pages are multi-level so perhaps that's required.

> +	dev_info(aic->dev, "%s\n", devname);

It's useful to display something like a device revision but if you're
just announcing that the driver has been loaded that's not so useful and
should be removed.  Similarly for the others.

> +#ifdef CONFIG_MFD_AIC_IRQ
> +	if (aic->irq) {
> +		ret = aic_irq_init(aic);
> +		if (ret < 0)
> +			goto err_irq;
> +	}
> +#endif

This doesn't look like it should be a config option?  If it should be
then these should be done with inline functions in the headers for stubs
rather than ifdefs in the code.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)