Re: [PATCH] xc5000, fix fw upload crash

[Devin Heitmueller <dheitmueller@kernellabs.com>]

On Tue, May 17, 2011 at 12:23 AM, Dmitri Belimov <d.belimov@gmail.com> wrote:
> Hi
>
> Fix crash when init tuner and upload twice the firmware into xc5000 at the some time.
>
> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
> index aa1b2e8..a491a5b 100644
> --- a/drivers/media/common/tuners/xc5000.c
> +++ b/drivers/media/common/tuners/xc5000.c
> @@ -996,6 +996,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>        struct xc5000_priv *priv = fe->tuner_priv;
>        int ret = 0;
>
> +       mutex_lock(&xc5000_list_mutex);
> +
>        if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
>                ret = xc5000_fwupload(fe);
>                if (ret != XC_RESULT_SUCCESS)
> @@ -1015,6 +1017,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>        /* Default to "CABLE" mode */
>        ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
>
> +       mutex_unlock(&xc5000_list_mutex);
> +
>        return ret;
>  }
>
>
> Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov <d.belimov@gmail.com>
>
> With my best regards, Dmitry.

NACK!

I don't think this patch is correct.  Concurrency problems are
expected to be handled in the upper layers, as there are usually much
more significant problems than just this case.  For example, if this
is a race between V4L2 and DVB, it is the responsibility of bridge
driver to provide proper locking.

If patches like this were accepted, we would need to litter every call
of all the tuner drivers with mutex lock/unlock, and it simply isn't
maintainable.

NACK unless Dmitri can provide a much better explanation as to why
this patch should be accepted rather than fixing the problem in the
bridge driver.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com