[PATCH 0/2] Hauppauge Solo/Dual HD spectral inversion

[PATCH 0/2] Hauppauge Solo/Dual HD spectral inversion

[Brad Love]

This is a misplaced addendum to my series:
- Hauppauge em28xx/lgdt3306a soloHD/DualHD support


This patch set requires:
https://patchwork.linuxtv.org/patch/46326/
https://patchwork.linuxtv.org/patch/46330/
https://patchwork.linuxtv.org/patch/46327/
https://patchwork.linuxtv.org/patch/46334/
https://patchwork.linuxtv.org/patch/46333/
https://patchwork.linuxtv.org/patch/46331/
https://patchwork.linuxtv.org/patch/46328/
https://patchwork.linuxtv.org/patch/46335/
https://patchwork.linuxtv.org/patch/46332/


This adds a spectrum inversion property to si2168
frontend configuration. Hauppauge Solo/Dual HD DVB
models have si2157 which produces inverted spectrum,
so they enable the property.


Brad Love (2):
  si2168: Add spectrum inversion property
  em28xx: Enable spectrum inversion for Hauppauge Solo/Dual HD DVB

 drivers/media/dvb-frontends/si2168.c  | 2 ++
 drivers/media/dvb-frontends/si2168.h  | 3 +++
 drivers/media/usb/em28xx/em28xx-dvb.c | 2 ++
 3 files changed, 7 insertions(+)

-- 
2.7.4

[PATCH 1/2] si2168: Add spectrum inversion property

[Brad Love]

Some tuners produce inverted spectrum, but the si2168 is not
currently set up to accept it. This adds an optional parameter
to set the frontend up to receive inverted spectrum.

Parameter is optional and only boards who enable inversion
will utilize this.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/dvb-frontends/si2168.c | 2 ++
 drivers/media/dvb-frontends/si2168.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index c041e79..46a597f 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -339,6 +339,8 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 
 	memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
 	cmd.args[4] = delivery_system | bandwidth;
+	if (config->spectral_inversion)
+		cmd.args[5] |= 1;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
 	ret = si2168_cmd_execute(client, &cmd);
diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index f48f0fb..978854f 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -46,6 +46,9 @@ struct si2168_config {
 
 	/* TS clock gapped */
 	bool ts_clock_gapped;
+
+	/* Inverted spectrum */
+	bool spectral_inversion;
 };
 
 #endif
-- 
2.7.4

[PATCH 2/2] em28xx: Enable inversion for Solo/Dual HD DVB models

[Brad Love]

Hauppauge Solo/Dual HD DVB models use a si2157 tuner, which is set to
produce inverted spectrum. This configures the si2168 DVB demod for
inverted spectrum on both affected models.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/usb/em28xx/em28xx-dvb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 9bc9576..6d0616e 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1727,6 +1727,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 			si2168_config.i2c_adapter = &adapter;
 			si2168_config.fe = &dvb->fe[0];
 			si2168_config.ts_mode = SI2168_TS_PARALLEL;
+			si2168_config.spectral_inversion = true;
 			memset(&info, 0, sizeof(struct i2c_board_info));
 			strlcpy(info.type, "si2168", I2C_NAME_SIZE);
 			info.addr = 0x64;
@@ -1911,6 +1912,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 			si2168_config.i2c_adapter = &adapter;
 			si2168_config.fe = &dvb->fe[0];
 			si2168_config.ts_mode = SI2168_TS_SERIAL;
+			si2168_config.spectral_inversion = true;
 			memset(&info, 0, sizeof(struct i2c_board_info));
 			strlcpy(info.type, "si2168", I2C_NAME_SIZE);
 			if (dev->ts == PRIMARY_TS)
-- 
2.7.4

[PATCH v2 1/2] si2168: Add spectrum inversion property

[Brad Love]

Some tuners produce inverted spectrum, but the si2168 is not
currently set up to accept it. This adds an optional parameter
to set the frontend up to receive inverted spectrum.

Parameter is optional and only boards who enable inversion
will utilize this.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v1:
- Embarassing build failure due to missing declaration.

 drivers/media/dvb-frontends/si2168.c | 3 +++
 drivers/media/dvb-frontends/si2168.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index c041e79..048b815 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -213,6 +213,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	struct i2c_client *client = fe->demodulator_priv;
 	struct si2168_dev *dev = i2c_get_clientdata(client);
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct si2168_config *config = client->dev.platform_data;
 	int ret;
 	struct si2168_cmd cmd;
 	u8 bandwidth, delivery_system;
@@ -339,6 +340,8 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 
 	memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
 	cmd.args[4] = delivery_system | bandwidth;
+	if (config->spectral_inversion)
+		cmd.args[5] |= 1;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
 	ret = si2168_cmd_execute(client, &cmd);
diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index f48f0fb..d519edd 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -46,6 +46,9 @@ struct si2168_config {
 
 	/* TS clock gapped */
 	bool ts_clock_gapped;
+
+	/* Inverted spectrum */
+	bool spectral_inversion;
 };
 
 #endif
-- 
2.7.4

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Antti Palosaari]

On 01/17/2018 11:52 PM, Brad Love wrote:
> Some tuners produce inverted spectrum, but the si2168 is not
> currently set up to accept it. This adds an optional parameter
> to set the frontend up to receive inverted spectrum.
> 
> Parameter is optional and only boards who enable inversion
> will utilize this.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Changes since v1:
> - Embarassing build failure due to missing declaration.
> 
>   drivers/media/dvb-frontends/si2168.c | 3 +++
>   drivers/media/dvb-frontends/si2168.h | 3 +++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> index c041e79..048b815 100644
> --- a/drivers/media/dvb-frontends/si2168.c
> +++ b/drivers/media/dvb-frontends/si2168.c
> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
>   	struct i2c_client *client = fe->demodulator_priv;
>   	struct si2168_dev *dev = i2c_get_clientdata(client);
>   	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	struct si2168_config *config = client->dev.platform_data;

hmmm, are you sure platform data pointer points is const? I usually tend 
to store all config information to device state. Then there is no need 
to care if pointer is valid or not anymore.

And inversion happens when those wires are cross-connected

>   	int ret;
>   	struct si2168_cmd cmd;
>   	u8 bandwidth, delivery_system;
> @@ -339,6 +340,8 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
>   
>   	memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
>   	cmd.args[4] = delivery_system | bandwidth;
> +	if (config->spectral_inversion)
> +		cmd.args[5] |= 1;
>   	cmd.wlen = 6;
>   	cmd.rlen = 4;
>   	ret = si2168_cmd_execute(client, &cmd);
> diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
> index f48f0fb..d519edd 100644
> --- a/drivers/media/dvb-frontends/si2168.h
> +++ b/drivers/media/dvb-frontends/si2168.h
> @@ -46,6 +46,9 @@ struct si2168_config {
>   
>   	/* TS clock gapped */
>   	bool ts_clock_gapped;
> +
> +	/* Inverted spectrum */
> +	bool spectral_inversion;
>   };
>   
>   #endif
> 

-- 
http://palosaari.fi/

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Brad Love]

On 2018-01-17 16:02, Antti Palosaari wrote:
>
>
> On 01/17/2018 11:52 PM, Brad Love wrote:
>> Some tuners produce inverted spectrum, but the si2168 is not
>> currently set up to accept it. This adds an optional parameter
>> to set the frontend up to receive inverted spectrum.
>>
>> Parameter is optional and only boards who enable inversion
>> will utilize this.
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> Changes since v1:
>> - Embarassing build failure due to missing declaration.
>>
>>   drivers/media/dvb-frontends/si2168.c | 3 +++
>>   drivers/media/dvb-frontends/si2168.h | 3 +++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/dvb-frontends/si2168.c
>> b/drivers/media/dvb-frontends/si2168.c
>> index c041e79..048b815 100644
>> --- a/drivers/media/dvb-frontends/si2168.c
>> +++ b/drivers/media/dvb-frontends/si2168.c
>> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct
>> dvb_frontend *fe)
>>       struct i2c_client *client = fe->demodulator_priv;
>>       struct si2168_dev *dev = i2c_get_clientdata(client);
>>       struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +    struct si2168_config *config = client->dev.platform_data;
>
> hmmm, are you sure platform data pointer points is const? I usually
> tend to store all config information to device state. Then there is no
> need to care if pointer is valid or not anymore.
>
> And inversion happens when those wires are cross-connected

It just dawned on me that the platform_data is stack allocated and
therefore not safe to access outside of probe. I will fix this momentarily.

I was informed by one of our hardware guys that the two models in patch
2/2 are inverted spectrum, so I guess they have wires cross-connected. I
can verify this again to be sure.



>
>>       int ret;
>>       struct si2168_cmd cmd;
>>       u8 bandwidth, delivery_system;
>> @@ -339,6 +340,8 @@ static int si2168_set_frontend(struct
>> dvb_frontend *fe)
>>         memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
>>       cmd.args[4] = delivery_system | bandwidth;
>> +    if (config->spectral_inversion)
>> +        cmd.args[5] |= 1;
>>       cmd.wlen = 6;
>>       cmd.rlen = 4;
>>       ret = si2168_cmd_execute(client, &cmd);
>> diff --git a/drivers/media/dvb-frontends/si2168.h
>> b/drivers/media/dvb-frontends/si2168.h
>> index f48f0fb..d519edd 100644
>> --- a/drivers/media/dvb-frontends/si2168.h
>> +++ b/drivers/media/dvb-frontends/si2168.h
>> @@ -46,6 +46,9 @@ struct si2168_config {
>>         /* TS clock gapped */
>>       bool ts_clock_gapped;
>> +
>> +    /* Inverted spectrum */
>> +    bool spectral_inversion;
>>   };
>>     #endif
>>
>

[PATCH v3 1/2] si2168: Add spectrum inversion property

[Brad Love]

Some tuners produce inverted spectrum, but the si2168 is not
currently set up to accept it. This adds an optional parameter
to set the frontend up to receive inverted spectrum.

Parameter is optional and only boards who enable inversion
will utilize this.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v2:
- platform_data is not safe outside of probe, move property to state

Changes since v1:
- Embarassing build failure due to missing declaration.

 drivers/media/dvb-frontends/si2168.c      | 3 +++
 drivers/media/dvb-frontends/si2168.h      | 3 +++
 drivers/media/dvb-frontends/si2168_priv.h | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index c041e79..3306dc5 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -339,6 +339,8 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 
 	memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
 	cmd.args[4] = delivery_system | bandwidth;
+	if (dev->spectral_inversion)
+		cmd.args[5] |= 1;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
 	ret = si2168_cmd_execute(client, &cmd);
@@ -799,6 +801,7 @@ static int si2168_probe(struct i2c_client *client,
 	dev->ts_mode = config->ts_mode;
 	dev->ts_clock_inv = config->ts_clock_inv;
 	dev->ts_clock_gapped = config->ts_clock_gapped;
+	dev->spectral_inversion = config->spectral_inversion;
 
 	dev_info(&client->dev, "Silicon Labs Si2168-%c%d%d successfully identified\n",
 		 dev->version >> 24 & 0xff, dev->version >> 16 & 0xff,
diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index f48f0fb..d519edd 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -46,6 +46,9 @@ struct si2168_config {
 
 	/* TS clock gapped */
 	bool ts_clock_gapped;
+
+	/* Inverted spectrum */
+	bool spectral_inversion;
 };
 
 #endif
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 3c8746a..2d362e1 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -48,6 +48,7 @@ struct si2168_dev {
 	u8 ts_mode;
 	bool ts_clock_inv;
 	bool ts_clock_gapped;
+	bool spectral_inversion;
 };
 
 /* firmware command struct */
-- 
2.7.4

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Brad Love]

On 2018-01-17 16:08, Brad Love wrote:
> On 2018-01-17 16:02, Antti Palosaari wrote:
>>
>> On 01/17/2018 11:52 PM, Brad Love wrote:
>>> Some tuners produce inverted spectrum, but the si2168 is not
>>> currently set up to accept it. This adds an optional parameter
>>> to set the frontend up to receive inverted spectrum.
>>>
>>> Parameter is optional and only boards who enable inversion
>>> will utilize this.
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>> ---
>>> Changes since v1:
>>> - Embarassing build failure due to missing declaration.
>>>
>>>   drivers/media/dvb-frontends/si2168.c | 3 +++
>>>   drivers/media/dvb-frontends/si2168.h | 3 +++
>>>   2 files changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/media/dvb-frontends/si2168.c
>>> b/drivers/media/dvb-frontends/si2168.c
>>> index c041e79..048b815 100644
>>> --- a/drivers/media/dvb-frontends/si2168.c
>>> +++ b/drivers/media/dvb-frontends/si2168.c
>>> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct
>>> dvb_frontend *fe)
>>>       struct i2c_client *client = fe->demodulator_priv;
>>>       struct si2168_dev *dev = i2c_get_clientdata(client);
>>>       struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>> +    struct si2168_config *config = client->dev.platform_data;
>> hmmm, are you sure platform data pointer points is const? I usually
>> tend to store all config information to device state. Then there is no
>> need to care if pointer is valid or not anymore.
>>
>> And inversion happens when those wires are cross-connected
> It just dawned on me that the platform_data is stack allocated and
> therefore not safe to access outside of probe. I will fix this momentarily.
>
> I was informed by one of our hardware guys that the two models in patch
> 2/2 are inverted spectrum, so I guess they have wires cross-connected. I
> can verify this again to be sure.


Hello Antti,

I have confirmation. No 'cross-connected' / swapped differential pair
polarities (if that's what you meant) on the IF pins. The si2157
inverted spectrum output is configurable though, and Hauppauge
have the tuner set up to output inverted. Sounds like it was a decision
based on interoperability with older demods.

Cheers,

Brad



>>>       int ret;
>>>       struct si2168_cmd cmd;
>>>       u8 bandwidth, delivery_system;
>>> @@ -339,6 +340,8 @@ static int si2168_set_frontend(struct
>>> dvb_frontend *fe)
>>>         memcpy(cmd.args, "\x14\x00\x0a\x10\x00\x00", 6);
>>>       cmd.args[4] = delivery_system | bandwidth;
>>> +    if (config->spectral_inversion)
>>> +        cmd.args[5] |= 1;
>>>       cmd.wlen = 6;
>>>       cmd.rlen = 4;
>>>       ret = si2168_cmd_execute(client, &cmd);
>>> diff --git a/drivers/media/dvb-frontends/si2168.h
>>> b/drivers/media/dvb-frontends/si2168.h
>>> index f48f0fb..d519edd 100644
>>> --- a/drivers/media/dvb-frontends/si2168.h
>>> +++ b/drivers/media/dvb-frontends/si2168.h
>>> @@ -46,6 +46,9 @@ struct si2168_config {
>>>         /* TS clock gapped */
>>>       bool ts_clock_gapped;
>>> +
>>> +    /* Inverted spectrum */
>>> +    bool spectral_inversion;
>>>   };
>>>     #endif
>>>

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Antti Palosaari]

On 01/18/2018 03:58 AM, Brad Love wrote:
> 
> On 2018-01-17 16:08, Brad Love wrote:
>> On 2018-01-17 16:02, Antti Palosaari wrote:
>>>
>>> On 01/17/2018 11:52 PM, Brad Love wrote:
>>>> Some tuners produce inverted spectrum, but the si2168 is not
>>>> currently set up to accept it. This adds an optional parameter
>>>> to set the frontend up to receive inverted spectrum.
>>>>
>>>> Parameter is optional and only boards who enable inversion
>>>> will utilize this.
>>>>
>>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>>> ---
>>>> Changes since v1:
>>>> - Embarassing build failure due to missing declaration.
>>>>
>>>>    drivers/media/dvb-frontends/si2168.c | 3 +++
>>>>    drivers/media/dvb-frontends/si2168.h | 3 +++
>>>>    2 files changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/media/dvb-frontends/si2168.c
>>>> b/drivers/media/dvb-frontends/si2168.c
>>>> index c041e79..048b815 100644
>>>> --- a/drivers/media/dvb-frontends/si2168.c
>>>> +++ b/drivers/media/dvb-frontends/si2168.c
>>>> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct
>>>> dvb_frontend *fe)
>>>>        struct i2c_client *client = fe->demodulator_priv;
>>>>        struct si2168_dev *dev = i2c_get_clientdata(client);
>>>>        struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>>> +    struct si2168_config *config = client->dev.platform_data;
>>> hmmm, are you sure platform data pointer points is const? I usually
>>> tend to store all config information to device state. Then there is no
>>> need to care if pointer is valid or not anymore.
>>>
>>> And inversion happens when those wires are cross-connected
>> It just dawned on me that the platform_data is stack allocated and
>> therefore not safe to access outside of probe. I will fix this momentarily.
>>
>> I was informed by one of our hardware guys that the two models in patch
>> 2/2 are inverted spectrum, so I guess they have wires cross-connected. I
>> can verify this again to be sure.
> 
> 
> Hello Antti,
> 
> I have confirmation. No 'cross-connected' / swapped differential pair
> polarities (if that's what you meant) on the IF pins. The si2157
> inverted spectrum output is configurable though, and Hauppauge
> have the tuner set up to output inverted. Sounds like it was a decision
> based on interoperability with older demods.

yeah, that was what I was thinking for. That board single tuner and two 
demods which other demod does not support if spectrum inversion?

If there is just si2168 and si2157, you can set both to invert or both 
to non-invert - the end result is same.

Antti

-- 
http://palosaari.fi/

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Brad Love]

On 2018-01-17 20:36, Antti Palosaari wrote:
> On 01/18/2018 03:58 AM, Brad Love wrote:
>>
>> On 2018-01-17 16:08, Brad Love wrote:
>>> On 2018-01-17 16:02, Antti Palosaari wrote:
>>>>
>>>> On 01/17/2018 11:52 PM, Brad Love wrote:
>>>>> Some tuners produce inverted spectrum, but the si2168 is not
>>>>> currently set up to accept it. This adds an optional parameter
>>>>> to set the frontend up to receive inverted spectrum.
>>>>>
>>>>> Parameter is optional and only boards who enable inversion
>>>>> will utilize this.
>>>>>
>>>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>>>> ---
>>>>> Changes since v1:
>>>>> - Embarassing build failure due to missing declaration.
>>>>>
>>>>>    drivers/media/dvb-frontends/si2168.c | 3 +++
>>>>>    drivers/media/dvb-frontends/si2168.h | 3 +++
>>>>>    2 files changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/dvb-frontends/si2168.c
>>>>> b/drivers/media/dvb-frontends/si2168.c
>>>>> index c041e79..048b815 100644
>>>>> --- a/drivers/media/dvb-frontends/si2168.c
>>>>> +++ b/drivers/media/dvb-frontends/si2168.c
>>>>> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct
>>>>> dvb_frontend *fe)
>>>>>        struct i2c_client *client = fe->demodulator_priv;
>>>>>        struct si2168_dev *dev = i2c_get_clientdata(client);
>>>>>        struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>>>> +    struct si2168_config *config = client->dev.platform_data;
>>>> hmmm, are you sure platform data pointer points is const? I usually
>>>> tend to store all config information to device state. Then there is no
>>>> need to care if pointer is valid or not anymore.
>>>>
>>>> And inversion happens when those wires are cross-connected
>>> It just dawned on me that the platform_data is stack allocated and
>>> therefore not safe to access outside of probe. I will fix this
>>> momentarily.
>>>
>>> I was informed by one of our hardware guys that the two models in patch
>>> 2/2 are inverted spectrum, so I guess they have wires
>>> cross-connected. I
>>> can verify this again to be sure.
>>
>>
>> Hello Antti,
>>
>> I have confirmation. No 'cross-connected' / swapped differential pair
>> polarities (if that's what you meant) on the IF pins. The si2157
>> inverted spectrum output is configurable though, and Hauppauge
>> have the tuner set up to output inverted. Sounds like it was a decision
>> based on interoperability with older demods.
>
> yeah, that was what I was thinking for. That board single tuner and
> two demods which other demod does not support if spectrum inversion?
>
> If there is just si2168 and si2157, you can set both to invert or both
> to non-invert - the end result is same.
>
> Antti


I will check on the HVR975 tomorrow, if it's affected as well I'll
submit a patch. The lgdt3306a frontend is already set up for auto
spectrum inversion, so it is able handle the si2157 in either config.

Re: [PATCH v2 1/2] si2168: Add spectrum inversion property

[Brad Love]

On 2018-01-17 21:10, Brad Love wrote:
> On 2018-01-17 20:36, Antti Palosaari wrote:
>> On 01/18/2018 03:58 AM, Brad Love wrote:
>>> On 2018-01-17 16:08, Brad Love wrote:
>>>> On 2018-01-17 16:02, Antti Palosaari wrote:
>>>>> On 01/17/2018 11:52 PM, Brad Love wrote:
>>>>>> Some tuners produce inverted spectrum, but the si2168 is not
>>>>>> currently set up to accept it. This adds an optional parameter
>>>>>> to set the frontend up to receive inverted spectrum.
>>>>>>
>>>>>> Parameter is optional and only boards who enable inversion
>>>>>> will utilize this.
>>>>>>
>>>>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>>>>> ---
>>>>>> Changes since v1:
>>>>>> - Embarassing build failure due to missing declaration.
>>>>>>
>>>>>>    drivers/media/dvb-frontends/si2168.c | 3 +++
>>>>>>    drivers/media/dvb-frontends/si2168.h | 3 +++
>>>>>>    2 files changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/media/dvb-frontends/si2168.c
>>>>>> b/drivers/media/dvb-frontends/si2168.c
>>>>>> index c041e79..048b815 100644
>>>>>> --- a/drivers/media/dvb-frontends/si2168.c
>>>>>> +++ b/drivers/media/dvb-frontends/si2168.c
>>>>>> @@ -213,6 +213,7 @@ static int si2168_set_frontend(struct
>>>>>> dvb_frontend *fe)
>>>>>>        struct i2c_client *client = fe->demodulator_priv;
>>>>>>        struct si2168_dev *dev = i2c_get_clientdata(client);
>>>>>>        struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>>>>> +    struct si2168_config *config = client->dev.platform_data;
>>>>> hmmm, are you sure platform data pointer points is const? I usually
>>>>> tend to store all config information to device state. Then there is no
>>>>> need to care if pointer is valid or not anymore.
>>>>>
>>>>> And inversion happens when those wires are cross-connected
>>>> It just dawned on me that the platform_data is stack allocated and
>>>> therefore not safe to access outside of probe. I will fix this
>>>> momentarily.
>>>>
>>>> I was informed by one of our hardware guys that the two models in patch
>>>> 2/2 are inverted spectrum, so I guess they have wires
>>>> cross-connected. I
>>>> can verify this again to be sure.
>>>
>>> Hello Antti,
>>>
>>> I have confirmation. No 'cross-connected' / swapped differential pair
>>> polarities (if that's what you meant) on the IF pins. The si2157
>>> inverted spectrum output is configurable though, and Hauppauge
>>> have the tuner set up to output inverted. Sounds like it was a decision
>>> based on interoperability with older demods.
>> yeah, that was what I was thinking for. That board single tuner and
>> two demods which other demod does not support if spectrum inversion?
>>
>> If there is just si2168 and si2157, you can set both to invert or both
>> to non-invert - the end result is same.
>>
>> Antti
>
> I will check on the HVR975 tomorrow, if it's affected as well I'll
> submit a patch. The lgdt3306a frontend is already set up for auto
> spectrum inversion, so it is able handle the si2157 in either config.
>

The HVR975 should be good as currently submitted.

Cheers,

Brad