From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751569AbeEUUo5 (ORCPT ); Mon, 21 May 2018 16:44:57 -0400 Received: from mail-vk0-f43.google.com ([209.85.213.43]:39715 "EHLO mail-vk0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbeEUUoy (ORCPT ); Mon, 21 May 2018 16:44:54 -0400 X-Google-Smtp-Source: AB8JxZqkQfCLpDriKcd6CxqaBBG/vm16sc5WdNaB5zrbTe/YZ0n96D54CtaXsL3gHNIeyObPpmM9PNAzoV63E9CVQtU= MIME-Version: 1.0 In-Reply-To: <20180521171415.00c56487@vento.lan> References: <20180521193951.GA16659@embeddedor.com> <20180521171415.00c56487@vento.lan> From: Devin Heitmueller Date: Mon, 21 May 2018 16:44:52 -0400 Message-ID: Subject: Re: [media] duplicate code in media drivers To: Mauro Carvalho Chehab Cc: "Gustavo A. R. Silva" , Mauro Carvalho Chehab , Linux Media Mailing List , Linux Kernel , Devin Heitmueller Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> diff -u -p drivers/media/dvb-frontends/au8522_decoder.c /tmp/nothing/media/dvb-frontends/au8522_decoder.c >> --- drivers/media/dvb-frontends/au8522_decoder.c >> +++ /tmp/nothing/media/dvb-frontends/au8522_decoder.c >> @@ -280,14 +280,9 @@ static void setup_decoder_defaults(struc >> AU8522_TOREGAAGC_REG0E5H_CVBS); >> au8522_writereg(state, AU8522_REG016H, AU8522_REG016H_CVBS); >> >> - if (is_svideo) { >> /* Despite what the table says, for the HVR-950q we still need >> to be in CVBS mode for the S-Video input (reason unknown). */ >> /* filter_coef_type = 3; */ >> - filter_coef_type = 5; >> - } else { >> - filter_coef_type = 5; >> - } > > Better ask Devin about this (c/c). This was a case where the implementation didn't match the datasheet, and it wasn't clear why the filter coefficients weren't working properly. Essentially I should have labeled that as a TODO or FIXME when I disabled the "right" value and forced it to always be five. It was also likely that the filter coefficients would need to differ if taking video over the IF interface as opposed to CVBS/S-video, which is why I didn't want to get rid of the logic entirely. That said, the only product I've ever seen with the tda18271 mated to the au8522 will likely never be supported for analog video under Linux for unrelated reasons. That said, it's worked "good enough" since I wrote the code nine years ago, so if somebody wants to submit a patch to either get rid of the if() statement or mark it as a FIXME that will likely never actually get fixed, I wouldn't have an objection to either. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com