[2/4] rtl28xxu: swap frontend order for devices with slave demodulators
Commit Message
Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
Comments
I am not sure even idea of that. You didn't add even commit description,
like all the other patches too :( You should really start adding commit
messages explaining why and how commit is.
So the question is why that patch should be applied?
On the other-hand, how there is
if (fe->id == 1 && onoff) {
... as I don't remember any patch changing it to 0. I look my tree FE ID
is 0. Do you have some unpublished hacks?
Antti
On 12/13/2014 02:18 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> index ab48b5f..cdc342a 100644
> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -863,6 +863,7 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
>
> /* attach slave demodulator */
> if (priv->slave_demod == SLAVE_DEMOD_MN88472) {
> + struct dvb_frontend *tmp_fe;
> struct mn88472_config mn88472_config = {};
>
> mn88472_config.fe = &adap->fe[1];
> @@ -887,6 +888,12 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
> }
>
> priv->i2c_client_slave_demod = client;
> +
> + /* Swap frontend order */
> + tmp_fe = adap->fe[0];
> + adap->fe[0] = adap->fe[1];
> + adap->fe[1] = tmp_fe;
> +
> } else {
> struct mn88473_config mn88473_config = {};
>
> @@ -1373,7 +1380,7 @@ static int rtl2832u_frontend_ctrl(struct dvb_frontend *fe, int onoff)
>
> /* bypass slave demod TS through master demod */
> if (fe->id == 1 && onoff) {
> - ret = rtl2832_enable_external_ts_if(adap->fe[0]);
> + ret = rtl2832_enable_external_ts_if(adap->fe[1]);
> if (ret)
> goto err;
> }
>
On 12/13/2014 05:02 AM, Antti Palosaari wrote:
> I am not sure even idea of that. You didn't add even commit
> description, like all the other patches too :( You should really start
> adding commit messages explaining why and how commit is.
>
> So the question is why that patch should be applied?
Lots of legacy applications doesn't set the frontend number and use 0 by
default. For me to use w_scan I need this change. If that is reason good
enough I can amend that to the commit message and resend?
>
> On the other-hand, how there is
> if (fe->id == 1 && onoff) {
> ... as I don't remember any patch changing it to 0. I look my tree FE
> ID is 0. Do you have some unpublished hacks?
No hacks, it works for me that way.
>
> Antti
MvH
Benjamin Larsson
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/13/2014 01:09 PM, Benjamin Larsson wrote:
> On 12/13/2014 05:02 AM, Antti Palosaari wrote:
>> I am not sure even idea of that. You didn't add even commit
>> description, like all the other patches too :( You should really start
>> adding commit messages explaining why and how commit is.
>>
>> So the question is why that patch should be applied?
>
> Lots of legacy applications doesn't set the frontend number and use 0 by
> default. For me to use w_scan I need this change. If that is reason good
> enough I can amend that to the commit message and resend?
>
>>
>> On the other-hand, how there is
>> if (fe->id == 1 && onoff) {
>> ... as I don't remember any patch changing it to 0. I look my tree FE
>> ID is 0. Do you have some unpublished hacks?
>
> No hacks, it works for me that way.
Do you understand that code at all?
Now it is:
FE0 == (fe->id == 0) == RTL2832
FE1 == (fe->id == 1) == MN88472
you changed it to:
FE0 == (fe->id == 0) == MN88472
FE1 == (fe->id == 1) == RTL2832
Then there is:
/* bypass slave demod TS through master demod */
if (fe->id == 1 && onoff) {
ret = rtl2832_enable_external_ts_if(adap->fe[1]);
if (ret)
goto err;
}
After your change that code branch is taken when RTL2832 demod is
activated / used. Shouldn't TS bypass enabled just opposite, when
MN88472 is used....
Antti
On 12/13/2014 02:35 PM, Antti Palosaari wrote:
>
>
> Do you understand that code at all?
No I can't really say I understand all the workings of the media api.
>
> Now it is:
> FE0 == (fe->id == 0) == RTL2832
> FE1 == (fe->id == 1) == MN88472
>
> you changed it to:
> FE0 == (fe->id == 0) == MN88472
> FE1 == (fe->id == 1) == RTL2832
I thought the rtl2832u_frontend_attach() actually attached the devices.
Then the id's would have followed the frontend.
>
> Then there is:
>
> /* bypass slave demod TS through master demod */
> if (fe->id == 1 && onoff) {
> ret = rtl2832_enable_external_ts_if(adap->fe[1]);
> if (ret)
> goto err;
> }
>
> After your change that code branch is taken when RTL2832 demod is
> activated / used. Shouldn't TS bypass enabled just opposite, when
> MN88472 is used....
>
>
> Antti
>
This intent of the patch was for better backwards compatibility with old
software. This isn't strictly needed so consider the patch dropped.
MvH
Benjamin Larsson
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -863,6 +863,7 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
/* attach slave demodulator */
if (priv->slave_demod == SLAVE_DEMOD_MN88472) {
+ struct dvb_frontend *tmp_fe;
struct mn88472_config mn88472_config = {};
mn88472_config.fe = &adap->fe[1];
@@ -887,6 +888,12 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
}
priv->i2c_client_slave_demod = client;
+
+ /* Swap frontend order */
+ tmp_fe = adap->fe[0];
+ adap->fe[0] = adap->fe[1];
+ adap->fe[1] = tmp_fe;
+
} else {
struct mn88473_config mn88473_config = {};
@@ -1373,7 +1380,7 @@ static int rtl2832u_frontend_ctrl(struct dvb_frontend *fe, int onoff)
/* bypass slave demod TS through master demod */
if (fe->id == 1 && onoff) {
- ret = rtl2832_enable_external_ts_if(adap->fe[0]);
+ ret = rtl2832_enable_external_ts_if(adap->fe[1]);
if (ret)
goto err;
}