Re: [PATCH] af9035: remove I2C client differently

[Antti Palosaari <crope@iki.fi>]

V2
* Fixed trivial bug reported by Daniel
* if/if => if/if else. Compiler, very likely, could not see adap->id 
behind many pointers, cannot change between two consecutive if's. Adding 
exclusive if()/if else() makes sure compiler knows only one of if 
branches could taken.

Antti


On 09/05/2014 02:15 AM, Antti Palosaari wrote:
> It crash kernel when device was removed while it was streaming.
> That is because we removed driver and frontend thread was still
> running. Use new callback which allows I2C driver removal just
> after frontend is unregistered.
>
> V2: fixed by reported by Daniel
>
> Reported-by: Daniel Glöckner <daniel-gl@gmx.net>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>   drivers/media/usb/dvb-usb-v2/af9035.c | 93 +++++++++++++++++++++++++----------
>   1 file changed, 67 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 94563b2..440ecb4 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1074,15 +1074,13 @@ static int af9035_get_adapter_count(struct dvb_usb_device *d)
>   	return state->dual_mode + 1;
>   }
>
> -static void af9035_exit(struct dvb_usb_device *d);
> -
>   static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
>   {
>   	struct state *state = adap_to_priv(adap);
>   	struct dvb_usb_device *d = adap_to_d(adap);
>   	int ret;
>
> -	dev_dbg(&d->udev->dev, "%s:\n", __func__);
> +	dev_dbg(&d->udev->dev, "%s: adap->id=%d\n", __func__, adap->id);
>
>   	if (!state->af9033_config[adap->id].tuner) {
>   		/* unsupported tuner */
> @@ -1109,12 +1107,48 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
>   	return 0;
>
>   err:
> -	af9035_exit(d); /* remove I2C clients */
>   	dev_dbg(&d->udev->dev, "%s: failed=%d\n", __func__, ret);
>
>   	return ret;
>   }
>
> +static int af9035_frontend_detach(struct dvb_usb_adapter *adap)
> +{
> +	struct state *state = adap_to_priv(adap);
> +	struct dvb_usb_device *d = adap_to_d(adap);
> +	int demod2;
> +
> +	dev_dbg(&d->udev->dev, "%s: adap->id=%d\n", __func__, adap->id);
> +
> +	/*
> +	 * For dual tuner devices we have to resolve 2nd demod client, as there
> +	 * is two different kind of tuner drivers; one is using I2C binding
> +	 * and the other is using DVB attach/detach binding.
> +	 */
> +	switch (state->af9033_config[adap->id].tuner) {
> +	case AF9033_TUNER_IT9135_38:
> +	case AF9033_TUNER_IT9135_51:
> +	case AF9033_TUNER_IT9135_52:
> +	case AF9033_TUNER_IT9135_60:
> +	case AF9033_TUNER_IT9135_61:
> +	case AF9033_TUNER_IT9135_62:
> +		demod2 = 2;
> +		break;
> +	default:
> +		demod2 = 1;
> +	}
> +
> +	if (adap->id == 1) {
> +		if (state->i2c_client[demod2])
> +			af9035_del_i2c_dev(d);
> +	} else if (adap->id == 0) {
> +		if (state->i2c_client[0])
> +			af9035_del_i2c_dev(d);
> +	}
> +
> +	return 0;
> +}
> +
>   static struct tua9001_config af9035_tua9001_config = {
>   	.i2c_addr = 0x60,
>   };
> @@ -1174,7 +1208,7 @@ static int af9035_tuner_attach(struct dvb_usb_adapter *adap)
>   	struct i2c_msg msg[1];
>   	u8 tuner_addr;
>
> -	dev_dbg(&d->udev->dev, "%s:\n", __func__);
> +	dev_dbg(&d->udev->dev, "%s: adap->id=%d\n", __func__, adap->id);
>
>   	/*
>   	 * XXX: Hack used in that function: we abuse unused I2C address bit [7]
> @@ -1392,12 +1426,37 @@ static int af9035_tuner_attach(struct dvb_usb_adapter *adap)
>   	return 0;
>
>   err:
> -	af9035_exit(d); /* remove I2C clients */
>   	dev_dbg(&d->udev->dev, "%s: failed=%d\n", __func__, ret);
>
>   	return ret;
>   }
>
> +static int af9035_tuner_detach(struct dvb_usb_adapter *adap)
> +{
> +	struct state *state = adap_to_priv(adap);
> +	struct dvb_usb_device *d = adap_to_d(adap);
> +
> +	dev_dbg(&d->udev->dev, "%s: adap->id=%d\n", __func__, adap->id);
> +
> +	switch (state->af9033_config[adap->id].tuner) {
> +	case AF9033_TUNER_IT9135_38:
> +	case AF9033_TUNER_IT9135_51:
> +	case AF9033_TUNER_IT9135_52:
> +	case AF9033_TUNER_IT9135_60:
> +	case AF9033_TUNER_IT9135_61:
> +	case AF9033_TUNER_IT9135_62:
> +		if (adap->id == 1) {
> +			if (state->i2c_client[3])
> +				af9035_del_i2c_dev(d);
> +		} else if (adap->id == 0) {
> +			if (state->i2c_client[1])
> +				af9035_del_i2c_dev(d);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static int af9035_init(struct dvb_usb_device *d)
>   {
>   	struct state *state = d_to_priv(d);
> @@ -1445,25 +1504,6 @@ err:
>   	return ret;
>   }
>
> -static void af9035_exit(struct dvb_usb_device *d)
> -{
> -	struct state *state = d_to_priv(d);
> -
> -	dev_dbg(&d->udev->dev, "%s:\n", __func__);
> -
> -	if (state->i2c_client[3])
> -		af9035_del_i2c_dev(d);
> -
> -	if (state->i2c_client[2])
> -		af9035_del_i2c_dev(d);
> -
> -	if (state->i2c_client[1])
> -		af9035_del_i2c_dev(d);
> -
> -	if (state->i2c_client[0])
> -		af9035_del_i2c_dev(d);
> -}
> -
>   #if IS_ENABLED(CONFIG_RC_CORE)
>   static int af9035_rc_query(struct dvb_usb_device *d)
>   {
> @@ -1636,11 +1676,12 @@ static const struct dvb_usb_device_properties af9035_props = {
>   	.i2c_algo = &af9035_i2c_algo,
>   	.read_config = af9035_read_config,
>   	.frontend_attach = af9035_frontend_attach,
> +	.frontend_detach = af9035_frontend_detach,
>   	.tuner_attach = af9035_tuner_attach,
> +	.tuner_detach = af9035_tuner_detach,
>   	.init = af9035_init,
>   	.get_rc_config = af9035_get_rc_config,
>   	.get_stream_config = af9035_get_stream_config,
> -	.exit = af9035_exit,
>
>   	.get_adapter_count = af9035_get_adapter_count,
>   	.adapter = {
>

-- 
http://palosaari.fi/