Hi Hans, It looks like it should. I'm looking at things on my phone while out and about - not the best environment for code review.
You just need to ensure default vol is >= 0 and <= 65535 which may not be the case if reg 8d4 has a value near 0. Two other topics while I'm here: 1. Why set the vol step to 655, when the volume will actaully step at increments of 512? 2. Why should failure to initialize a data structure for user controls mean failure to init otherwise working hardware? We never let user control init cause subdev driver probe failure before, so why now? I'd prefer a working device without user controls in case of user control init failure. Regards, Andy Hans Verkuil <[email protected]> wrote: >On Sunday, December 12, 2010 19:09:36 Andy Walls wrote: >> Hans, >> >> This has at least the same two problems the change for cx25840 had: >> >> 1. Volume control init should use 65535 not 65335 >> >> 2. You cannot trust reg 0x8d4 to have a value in it for the default volume >> that won't give an ERANGE error when you go to init the volume control. >> Subdev probe will fail. See my previous cx25840 patches sent to the list, >> awaiting action. >> >> (The cx25840 code that trusts the volume register to be in range is already >> in 2.6.36 and breaks IR and analog for CX23885/7/8 chips.) >> >> I'll give this whole patch a harder look later this evening if I can. >> >> Regards, >> Andy >> > >Would the patch below fix these issues? It compiles, but I didn't test it. > >Regards, > > Hans > >diff --git a/drivers/media/video/cx18/cx18-av-core.c >b/drivers/media/video/cx18/cx18-av-core.c >index e1f58f1..73b6f4d 100644 >--- a/drivers/media/video/cx18/cx18-av-core.c >+++ b/drivers/media/video/cx18/cx18-av-core.c >@@ -248,8 +248,22 @@ static void cx18_av_initialize(struct v4l2_subdev *sd) > /* CxDevWrReg(CXADEC_SRC_COMB_CFG, 0x6628021F); */ > /* } */ > cx18_av_write4(cx, CXADEC_SRC_COMB_CFG, 0x6628021F); >- default_volume = 228 - cx18_av_read(cx, 0x8d4); >- default_volume = ((default_volume / 2) + 23) << 9; >+ default_volume = cx18_av_read(cx, 0x8d4); >+ /* >+ * Enforce the legacy volume scale mapping limits to avoid -ERANGE >+ * errors when initializing the volume control >+ */ >+ if (default_volume > 228) { >+ /* Bottom out at -96 dB, v4l2 vol range 0x2e00-0x2fff */ >+ default_volume = 228; >+ cx18_av_write(cx, 0x8d4, 228); >+ } >+ else if (default_volume < 20) { >+ /* Top out at + 8 dB, v4l2 vol range 0xfe00-0xffff */ >+ default_volume = 20; >+ cx18_av_write(cx, 0x8d4, 20); >+ } >+ default_volume = (((228 - default_volume) >> 1) + 23) << 9; > state->volume->cur.val = state->volume->default_value = default_volume; > v4l2_ctrl_handler_setup(&state->hdl); > } >@@ -1359,7 +1373,7 @@ int cx18_av_probe(struct cx18 *cx) > > state->volume = v4l2_ctrl_new_std(&state->hdl, > &cx18_av_audio_ctrl_ops, V4L2_CID_AUDIO_VOLUME, >- 0, 65335, 65535 / 100, 0); >+ 0, 65535, 65535 / 100, 0); > v4l2_ctrl_new_std(&state->hdl, > &cx18_av_audio_ctrl_ops, V4L2_CID_AUDIO_MUTE, > 0, 1, 1, 0); > >-- >Hans Verkuil - video4linux developer - sponsored by Cisco >-- >To unsubscribe from this list: send the line "unsubscribe linux-media" in >the body of a message to [email protected] >More majordomo info at http://vger.kernel.org/majordomo-info.html N�����r��y����b�X��ǧv�^�){.n�+����{���bj)����w* jg��� �����ݢj/���z�ޖ��2�ޙ����&�)ߡ�a���� �G���h��j:+v���w��٥
