On Tue, Sep 06, 2011 at 06:32:42PM +0800, johnnyhsu@realtek.com wrote:

switch (reg) {

• case RT5631_VENDOR_ID:
• case RT5631_VENDOR_ID1:
• case RT5631_VENDOR_ID2:

• return 0;
  

• default:

• case RT5631_RESET:
• case RT5631_INT_ST_IRQ_CTRL_2:
• case RT5631_INDEX_ADD:
• case RT5631_INDEX_DATA:
• case RT5631_EQ_CTRL: return 1;
• default:

• return 0;
  

This is an incremental patch against your previous driver, not a full patch. You should always send a patches which can be applied against current kernels.

• pr_info("enter %s, syclk=%d\n", __func__, freq);

• dev_info(codec->dev, "enter %s, syclk=%d\n", __func__, freq);

This should be a debug level print at most, otherwise you just spam the console.

if (!freq_in || !freq_out) { dev_dbg(codec->dev, "PLL disabled\n");

• return -EINVAL;
  

• return 0;
  

I'd expect to see some sort of register write when the PLL is disabled?

/**

• • rt5631_index_reg_show - sysfs file for dumping index registers of 2nd layer

• • rt5631_index_reg_show: Show private register of rt5631 codec.
• • @dev: device to query.
• • @attr: device attribute.
• • @buf: buffer to display.
• • To show private registers which are not changed often by user.
• • You have to access them through register 0x6a and 0x6c.
  */

static ssize_t rt5631_index_reg_show(struct device *dev, struct device_attribute *attr, char *buf)

As I've told you repeatedly you should be using the standard facilities for debug access to registers - you should not open coding this in your driver. Once again, ignoring review comments isn't helpful.