Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965437AbbBDORR (ORCPT ); Wed, 4 Feb 2015 09:17:17 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:57314 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932599AbbBDORO (ORCPT ); Wed, 4 Feb 2015 09:17:14 -0500 From: Laurent Pinchart To: Hans Verkuil Cc: Pablo Anton , hans.verkuil@cisco.com, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, mchehab@osg.samsung.com, lars@metafoo.de, Jean-Michel Hautbois Subject: Re: [PATCH] media: i2c: ADV7604: Rename adv7604 prefixes. Date: Wed, 04 Feb 2015 16:17:59 +0200 Message-ID: <3313129.aSJuHsSGG3@avalon> User-Agent: KMail/4.14.3 (Linux/3.17.7-gentoo; KDE/4.14.3; x86_64; ; ) In-Reply-To: <54D20406.9000300@xs4all.nl> References: <1422983598-9189-1-git-send-email-pablo.anton@vodalys-labs.com> <9008824.dHMd7MRA5e@avalon> <54D20406.9000300@xs4all.nl> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4605 Lines: 120 Hi Hans, On Wednesday 04 February 2015 12:35:34 Hans Verkuil wrote: > On 02/04/15 12:27, Laurent Pinchart wrote: > > On Wednesday 04 February 2015 10:55:21 Hans Verkuil wrote: > >> On 02/03/15 18:13, Pablo Anton wrote: > >>> It is confusing which parts of the driver are adv7604 specific, adv7611 > >>> specific or common for both. This patch renames any adv7604 prefixes > >>> (both for functions and defines) to adv76xx whenever they are common. > >>> > >>> Signed-off-by: Pablo Anton > >>> Signed-off-by: Jean-Michel Hautbois > >> > >> I'm happy with this, except for three small changes: > >> > >> - I had to rebase > >> - ADV76xx_fsc should be ADV76XX_FSC > >> - The driver name should stay the same to keep in sync with the module > >> name. Besides, we might have a future driver for the adv7622/3, so > >> adv76xx as the driver name is potentially confusing. > >> > >> I've applied these changes and the updated patch is below. If possible I > >> would like to get this in 3.20 so future patches for 3.21 can all be > >> based on these renamed functions/defines. > >> > >> Acks from Lars and Laurent would be welcome, though. > >> > >> Regards, > >> > >> Hans > >> > >> From bff6f026de4fe276f99be6ca38206720659938dc Mon Sep 17 00:00:00 2001 > >> From: Pablo Anton > >> Date: Tue, 3 Feb 2015 18:13:18 +0100 > >> Subject: [PATCH] media: i2c: ADV7604: Rename adv7604 prefixes. > >> > >> It is confusing which parts of the driver are adv7604 specific, adv7611 > >> specific or common for both. This patch renames any adv7604 prefixes > >> (both for functions and defines) to adv76xx whenever they are common. > >> > >> Signed-off-by: Pablo Anton > >> Signed-off-by: Jean-Michel Hautbois > >> [hans.verkuil@cisco.com: rebased and renamed ADV76xx_fsc to ADV76XX_FSC] > >> [hans.verkuil@cisco.com: kept the existing adv7604 driver name] > >> Signed-off-by: Hans Verkuil > >> --- > >> > >> drivers/media/i2c/adv7604.c | 898 ++++++++++++++++++------------------- > >> include/media/adv7604.h | 83 ++-- > >> 2 files changed, 491 insertions(+), 490 deletions(-) > > > > [snip] > > > >> diff --git a/include/media/adv7604.h b/include/media/adv7604.h > >> index aa1c447..9ecf353 100644 > >> --- a/include/media/adv7604.h > >> +++ b/include/media/adv7604.h > >> @@ -47,16 +47,16 @@ enum adv7604_bus_order { > > > > [snip] > > > >> -enum adv7604_page { > >> - ADV7604_PAGE_IO, > >> +enum adv76xx_page { > >> + ADV76XX_PAGE_IO, > >> ADV7604_PAGE_AVLINK, > >> - ADV7604_PAGE_CEC, > >> - ADV7604_PAGE_INFOFRAME, > >> + ADV76XX_PAGE_CEC, > >> + ADV76XX_PAGE_INFOFRAME, > >> ADV7604_PAGE_ESDP, > >> ADV7604_PAGE_DPP, > >> - ADV7604_PAGE_AFE, > >> - ADV7604_PAGE_REP, > >> - ADV7604_PAGE_EDID, > >> - ADV7604_PAGE_HDMI, > >> - ADV7604_PAGE_TEST, > >> - ADV7604_PAGE_CP, > >> + ADV76XX_PAGE_AFE, > >> + ADV76XX_PAGE_REP, > >> + ADV76XX_PAGE_EDID, > >> + ADV76XX_PAGE_HDMI, > >> + ADV76XX_PAGE_TEST, > >> + ADV76XX_PAGE_CP, > >> ADV7604_PAGE_VDP, > >> - ADV7604_PAGE_MAX, > >> + ADV76XX_PAGE_MAX, > >> }; > > > > (Taking the above chunk as one particular example, the comment applies to > > the rest of the driver.) > > > > I'm fine with the change in general, but I wonder how we will handle it > > going forward. Here the ADV7604-specific pages keep their ADV7604_ > > prefix, while the pages common to all supported chips now use an ADV76XX_ > > prefix. If a new chip comes out tomorrow with support, let's say, for > > AVLINK, how will you name ADV7604_PAGE_AVLINK ? Renaming it to > > ADV76XX_PAGE_AVLINK would imply that it's supported on all chips, which > > wouldn't be true, and keeping the existing name would imply that it's > > only supported on the ADV7604, which wouldn't be true either. > > I'd probably choose something like: ADV7604_12_PAGE_AVLINK if this was > supported for e.g. the ADV7604 and ADV7612, but not ADV7611. > > More likely would be scenarios where registers are supported for the adv761x > but not for the adv7604, and in that case it would be ADV761X of course. I guess it will be a wait-and-see kind of situation. I'm fine with the patch if you believe it improves readability. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/