Messages in this thread | | | Date | Wed, 1 May 2013 16:43:46 +0100 | From | Mark Brown <> | 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. [unhandled content-type:application/pgp-signature] | |