From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH v2] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Date: Tue, 13 Feb 2018 17:06:19 +0200 Message-ID: References: <20180127133209.28995-1-pali.rohar@gmail.com> <20180128144509.pobnj7cayc4psgrj@pali> <20180131120348.azy25aqvn5wrdkeh@pali> <20180212153012.vffvjmz26ifyxbj5@pali> <20180213150004.5d2v7y7wwuure4io@pali> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20180213150004.5d2v7y7wwuure4io@pali> Sender: platform-driver-x86-owner@vger.kernel.org To: =?UTF-8?Q?Pali_Roh=C3=A1r?= Cc: Jean Delvare , Wolfram Sang , =?UTF-8?B?TWljaGHFgiBLxJlwaWXFhA==?= , Steven Honeyman , Valdis Kletnieks , Jochen Eisinger , Gabriele Mazzotta , Andy Lutomirski , Mario Limonciello , Alex Hung , Takashi Iwai , linux-i2c , Linux Kernel Mailing List , Platform Driver List-Id: linux-i2c@vger.kernel.org On Tue, Feb 13, 2018 at 5:00 PM, Pali Roh=C3=A1r wro= te: > On Tuesday 13 February 2018 16:55:00 Andy Shevchenko wrote: >> On Mon, Feb 12, 2018 at 5:30 PM, Pali Roh=C3=A1r = wrote: >> > On Wednesday 31 January 2018 14:27:51 Andy Shevchenko wrote: >> >> On Wed, Jan 31, 2018 at 2:03 PM, Pali Roh=C3=A1r wrote: >> >> > On Sunday 28 January 2018 17:00:35 Andy Shevchenko wrote: >> >> >> On Sun, Jan 28, 2018 at 4:45 PM, Pali Roh=C3=A1r wrote: >> >> >> >> >> > ACPI device name is SMO8800, SMO8810, ... Will that acpi_dev_pre= sent >> >> >> > function match only prefix and not exact string? >> >> >> >> >> >> OK, fair enough. >> >> >> >> >> >> Do we have more users of such pattern? >> >> > >> >> > I have not seen this ACPI pattern yet, so probably not. >> >> >> >> I see. So, my one concern is the implicit names of the devices. I >> >> would like rather to see >> >> >> >> ... acpi_device_id ... []=3D { >> >> {"SMO8800"}, >> >> {"SMO8810"}, >> >> ... >> >> {} >> >> }; >> > >> > Following table already exists in dell-smo8800.c file: >> > >> > static const struct acpi_device_id smo8800_ids[] =3D { >> > { "SMO8800", 0 }, >> > { "SMO8801", 0 }, >> > { "SMO8810", 0 }, >> > { "SMO8811", 0 }, >> > { "SMO8820", 0 }, >> > { "SMO8821", 0 }, >> > { "SMO8830", 0 }, >> > { "SMO8831", 0 }, >> > { "", 0 }, >> > }; >> > >> > MODULE_DEVICE_TABLE(acpi, smo8800_ids); >> > >> > Can we reuse it? >> >> > Maybe moving array smo8800_ids[] into some header file >> > (which one?) and statically inline it? >> >> Bad idea. >> >> > Or having it only in >> > dell-smo8800.c file and exporting its symbol? >> >> Even worse. >> >> > Or is there better idea? >> > >> > For sure I do not want to copy paste this table into another module an= d >> > maintaining two copies of this list. >> >> The copy is fine. Can you guarantee that those two lists would be >> always the same? I'm not. > > Me neither. > >> And besides that explicitly over implicitly is a really good thing. I >> would not like to grep for an ID followed by grepping include line and >> check each files to check if it uses it or not. > > So what do you suggest now? Copy'n'paste and maintain two lists. Yes, it's not the ideal, but working solution. You may put a comment before each list to explain what the second does and tell a contributor to look at it and update if needed. > Having one file where it would be defined is a bad idea for you. Not just "one file", but "one *header* file". Or "exporting a symbol" which is basically not supposed to be exported. ID tables are part of the actual drivers, neither headers, nor libraries. > And maintaining copy of same array in two different files in two > different subsystems is something which I cannot guarantee. > > Therefore the current patch is the best approach. I don't like it. I'll not take it, sorry. > No shared file with > shared array/table and also no copy of that array in two different > subsystems. --=20 With Best Regards, Andy Shevchenko