Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Cinergy S2 USB Rev.4 #2353

Merged
merged 1 commit into from Jan 24, 2018
Merged

Conversation

dcervi
Copy link

@dcervi dcervi commented Dec 23, 2017

This restores support for Cinergy S2 USB Rev.4 DVB-S2 tuner. Now Cinergy S2 USB Rev.4 is working, and the new tuners added by this patch should be working as well. The modification only reverts the removal of the lines needed to support my tuner.

This is the first time I submit a pull request, so maybe I made something wrong in the process. I hope it can be reviewed and merged, so I and anyone using this tuner can return to official builds again.

@chewitt
Copy link
Member

chewitt commented Dec 26, 2017

@dcervi thanks for the submission, but no more 8.2 releases are planned. Is the change still valid against the master branch? - please test with a current milhouse 9.0/Leia build.

@dcervi
Copy link
Author

dcervi commented Dec 27, 2017

@chewitt The master branch now changed the way DVB modules are loaded (now installed as addons), but I still think this should be merged to the 8.2 branch. Most users will stay in 8.2 until Leia reaches beta or stable status. This should also benefit other community builds like current Amlogic ones using the libreelec-8.2 branch.

@chewitt chewitt requested a review from CvH December 27, 2017 12:14
@chewitt
Copy link
Member

chewitt commented Dec 27, 2017

@CvH please merge, request rework or close as you see fit, there are no plans for more 8.2 releases and it's not my area of expertise so I have no opinion on it in the release branch.

@CvH
Copy link
Member

CvH commented Jan 3, 2018

I am closing this as we don't plan to release another 8.2.x update (and its already fixed at LE9).
Tx @dcervi for pr.

@CvH CvH closed this Jan 3, 2018
@dcervi
Copy link
Author

dcervi commented Jan 23, 2018

@CvH It's sad this simple bugfix PR was closed without being merged. Now there's a new 8.2 release and this DVB tuner isn't supported. I know it's fixed at LE9 (I hadn't found time to test it yet), but LE9 is still in early Alpha stages, and I think this should be merged so if any other release comes this tuner would be supported also on LE8.2 for everyone.

Can you consider re-opening the PR and merging the commit, if it's not too much work, please?

@CvH
Copy link
Member

CvH commented Jan 23, 2018

@dcervi we didn't considered to release an 8.2.3 at the point of closing that pr - the main problem is that we have no real idea if it breaks something else.
Every release could be the last and I really fear that it might break existing working setups. As tiny this fix looks like it might be possible that it breaks the other cinergy revs. We want to prevent that anything breaks while updating (autoupgrade is quite common). We can't test this at all.
As said we are absolutely unsure if we release another 8.2.x or not - so if this patch breaks something it keeps broken :/

@dcervi
Copy link
Author

dcervi commented Jan 23, 2018

@CvH This was talked with Crazycat69. He mistakenly thought Cinergy S2 Rev. 4 was some DVBSky S960 clone and removed device descriptors from dw2102 driver. When his changes where merged on LE this previously supported tuner stopped working.

This patch only reverses the removal, and should only have effect on the specific VID-PID of this tuner. Even Crazycat69 made this commit on his latest branch to reverse the removal of this descriptors, but unfortunately this wasn't updated on LE8.2

For me it's a little inconvenience as I have to recompile every LE8.2 release and maybe this will be the latest, but if any new user coming to LE has this tuner he would find it isn't supported on the stable release. Personally I found my otherwise almost perfect tuner stopped working after some LE8 automatic update and I have spent many hours trying to find the cause and learning how to modify a patch and submit a PR on Github. I know it's only a little grain of sand in the enormous project LE is, but I hoped my work would help other users having the same problem.

Anyway I understand your concerns. I'm almost sure this won't affect any other device, but if you think otherwise, please feel free to follow your criteria.

@chewitt chewitt reopened this Jan 24, 2018
@chewitt chewitt self-requested a review January 24, 2018 02:16
@chewitt chewitt merged commit bf824be into LibreELEC:libreelec-8.2 Jan 24, 2018
@chewitt
Copy link
Member

chewitt commented Jan 24, 2018

There's no promise of an 8.2.4 release, but..

@ghost
Copy link

ghost commented Jan 25, 2018

ping @dcervi, $ALL
is it sure that Cinergy S2 USB Rev.4 is full supported by LibreELEC / vanilla kernel ?

Background:
after reading the $INTERNET I was confident that Cinergy S2 USB Rev.4 is supported by LibreELEC, so got one 2 days ago and plugged it in.

Rebooted my NUC and got a kernel crash with last release LibreELEC, sort of [1] !

So I plugged it in my Desktop PC running vanilla 4.15-rc9 and I got a kernel crash also !

I decided to it send the Cinergy back to the reseller.

Then I saw this conversation.
So I gave a last try by changing the related files in my 4.15-rc9 kernel tree.
The crash was fixed but I can't get the Cinergy running...

Under Windows 7 the Cinergy was running fine, so I exclude hardware related bugs.

It was a "ID 0ccd:0105 TerraTec Electronic GmbH" as noticed here:
https://linuxtv.org/wiki/index.php/TerraTec_Cinergy_S2_R2

[1]
https://bugzilla.kernel.org/show_bug.cgi?id=196967
or
https://bugzilla.kernel.org/show_bug.cgi?id=195029

@dcervi
Copy link
Author

dcervi commented Jan 25, 2018

@sixpack57 This only fixes a bug that appeared on a media_build update affecting some LE 8.1.X and LE 8.2.X releases. 'media_build' is a package including drivers for many DVB devices. I know it's used by Raspberry Pi and Amlogic devices, but I don't know about x86/x64 releases as I've never used them.

@dcervi dcervi deleted the Fix-Cinergy-S2-Rev4 branch January 25, 2018 23:43
@ghost
Copy link

ghost commented Jan 26, 2018

Thanks for info !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants