Re: [alsa-devel] [PATCH 13/31] ASoC: wm_adsp: Fix BUG_ON() and WARN_ON() usages

On Tue, Nov 05, 2013 at 06:40:00PM +0100, Takashi Iwai wrote:

@@ -645,27 +647,22 @@ static int wm_adsp_load(struct wm_adsp *dsp) reg = offset; break; case WMFW_ADSP1_PM:

• 	BUG_ON(!mem);
  region_name = "PM";
  reg = wm_adsp_region_to_reg(mem, offset);
  break;
  

  case WMFW_ADSP1_DM:

• 	BUG_ON(!mem);
  region_name = "DM";
  reg = wm_adsp_region_to_reg(mem, offset);
  break;
  

  case WMFW_ADSP2_XM:

• 	BUG_ON(!mem);
  region_name = "XM";
  reg = wm_adsp_region_to_reg(mem, offset);
  break;
  

  case WMFW_ADSP2_YM:

• 	BUG_ON(!mem);
  region_name = "YM";
  reg = wm_adsp_region_to_reg(mem, offset);
  break;
  

  case WMFW_ADSP1_ZM:

• 	BUG_ON(!mem);
  region_name = "ZM";
  reg = wm_adsp_region_to_reg(mem, offset);
  break;
  

These do make me a little nervous previously we would panic here, so everything would stop but now we may write random registers on the CODEC which could cause some odd behaviour. I think it would be better to update wm_adsp_region_to_reg to allow a failed return code and then error out of wm_adsp_load rather than just relying on the WARN_ON inside.

Otherwise the patch looks good to me though.

Thanks, Charles