lkml.org 
[lkml]   [2013]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2013-05-01 18:01    [W:0.084 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site