Re: [PATCHv3 1/7] OMAP4: PMIC: Add support for twl6030 codec

[Peter Ujfalusi <peter.ujfalusi@nokia.com>]

On Wednesday 07 October 2009 08:20:35 ext Krishnamoorthy, Balaji T wrote:
> > >  #define twl_has_mmc()   false
> > >  #endif
> > >
> > > +#if defined(CONFIG_SND_SOC_TWL6030) ||
> 
> defined(CONFIG_SND_SOC_TWL6030_MODULE) ||
> defined(CONFIG_SND_SOC_TWL4030) ||
> defined(CONFIG_SND_SOC_TWL4030_MODULE)
> 
> > > +#define twl_has_codec()	true
> > > +#else
> > > +#define twl_has_codec()	false
> > > +#endif
> > > +
> > >  #if defined(CONFIG_TWL4030_USB) || defined(CONFIG_TWL4030_USB_MODULE)
> > >  #define twl_has_usb()	true
> > >  #else
> > > @@ -190,6 +196,7 @@
> > >  #define BCI_SUB_CHIP_ID		SUB_CHIP_ID1
> > >  #define GPIO_SUB_CHIP_ID	0 /* NOT SUPPORTED IN TWL6030 */
> > >  #define KEYPAD_SUB_CHIP_ID	0 /* ADDED FOR COMPILATION ONLY */
> > > +#define CODEC_SUB_CHIP_ID	SUB_CHIP_ID3
> >
> > TWL4030 codec is under ID2 address group.
> 
> To pick the sub chip id from structure twl_mapping twl6030_map or
>  twl4030_map at Runtime.

Yes, it can be done.

> 
> > >  /* subchip/slave 0 0x48 - POWER */
> > >  #define TWL6030_BASEADD_RTC		0x0000
> > > @@ -632,6 +639,14 @@ add_children(struct twl_platform_data *pdata,
> > > unsigned long features) if (IS_ERR(child))
> > >  			return PTR_ERR(child);
> > >  	}
> > > +
> > > +	if (twl_has_codec()) {
> > > +		child = add_child(CODEC_SUB_CHIP_ID, "twl6030_codec",
> > > +			pdata->codec, sizeof(*pdata->codec), false,
> > > +			0, 0);
> > > +		if (IS_ERR(child))
> > > +			return PTR_ERR(child);
> > > +	}
> 
> after [PATCH v3 0/4] TWL6030 patch series
> Will now become something like
> +	if (twl_has_codec() && twl_class_is_6030()) {
> +		sub_chip_id = twl_map[TWL_MODULE_AUDIO_VOICE].sid;
> +		child = add_child(sub_chip_id, "twl6030_codec",
> +			pdata->codec, sizeof(*pdata->codec), false,
> +			0, 0);
> +		if (IS_ERR(child))
> +			return PTR_ERR(child);
> +	}
> And similarly for 4030 code
> +	if (twl_has_codec() && twl_class_is_4030()) {
> +		sub_chip_id = twl_map[TWL_MODULE_AUDIO_VOICE].sid;
> +		child = add_child(sub_chip_id, "twl4030_codec",
> +			pdata->codec, sizeof(*pdata->codec), false,
> +			0, 0);
> +		if (IS_ERR(child))
> +			return PTR_ERR(child);
> +	}

Sounds good.

> 
> > We are going to have the twl4030 as child as well.
> >
> > >  #endif
> > >
> > >  	if (twl_has_usb() && pdata->usb) {
> > > diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> > > index b687a8b..e76ca9b 100644
> > > --- a/include/linux/i2c/twl.h
> > > +++ b/include/linux/i2c/twl.h
> > > @@ -472,6 +472,12 @@ struct twl_usb_data {
> > >  	enum twl_usb_mode	usb_mode;
> > >  };
> > >
> > > +struct twl_codec_data {
> 
> 4030 and 6030 codec common fields can be added here
> 
> > > +	/* twl6030 */
> > > +	int audpwron_gpio;	/* audio power-on gpio */
> > > +	int naudint_irq;	/* audio interrupt */
> 
> Here twl4030 specific fields can be added

We can also use different twl_codec_data for 6030 and 4030.
There are not that many common things to handle, or is there?

> 
> > > +};
> 
> CONFIG_TWL6030_CORE and CONFIG_TWL4030_CORE is merged to CONFIG_TWL_CORE
> As two different build config was not encouraged.  Now switch
> twl_class_is_6030() will be used to decide at runtime time to support for
>  6030 related drivers, Similarly twl_class_is_4030() for 4030 drivers

I will hold back the patches for the twl4030 codec and twl core for now and wait 
till things settled a bit. 

Thanks,
Péter