From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935193AbdKPRJs (ORCPT ); Thu, 16 Nov 2017 12:09:48 -0500 Received: from lelnx193.ext.ti.com ([198.47.27.77]:65021 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934929AbdKPRJi (ORCPT ); Thu, 16 Nov 2017 12:09:38 -0500 Subject: Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1 To: "H. Nikolaus Schaller" CC: Tomi Valkeinen , Thierry Reding , David Airlie , Rob Herring , Mark Rutland , =?UTF-8?Q?Beno=c3=aet_Cousson?= , Tony Lindgren , Russell King , Bartlomiej Zolnierkiewicz , Laurent Pinchart , Julia Lawall , Sean Paul , , , , , , , , References: <474bfc72b67e1d0e2a223084a69f04d2baa1dda8.1510822218.git.hns@goldelico.com> <4753a425-d050-7f2d-35ae-b387accfd99e@ti.com> <130D3B71-C0CE-4755-BF06-971681F630BC@goldelico.com> <908C52B9-83E2-49B3-B5B4-D8F27A96ECD6@goldelico.com> From: "Andrew F. Davis" Message-ID: Date: Thu, 16 Nov 2017 11:08:30 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <908C52B9-83E2-49B3-B5B4-D8F27A96ECD6@goldelico.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote: > Hi Andrew, > >> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis : >> >> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote: >>> >>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen : >>>> >>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote: >>>>> The vendor name was "toppoly" but other panels and the vendor list >>>>> have defined it as "tpo". So let's fix it in driver and bindings. >>>>> >>>>> Signed-off-by: H. Nikolaus Schaller >>>>> --- >>>> >>>> >>>>> -MODULE_ALIAS("spi:toppoly,td028ttec1"); >>>>> +MODULE_ALIAS("spi:tpo,td028ttec1"); >>>> >>>> Doesn't this mean that the module won't load if you have old bindings? >>> >>> Hm. >>> >>> Well, I think it can load but doesn't automatically from DT strings which might >>> be unexpected. >>> >>>> Can't we have two module aliases? >>> >>> I think we can. Just a random example: >>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754 >>> >>> So we should keep both. >> >> Even better would be to drop both MODULE_ALIAS and let the >> MODULE_DEVICE_TABLE macro define them for your from the SPI id table. > > Why would that be better? > MODULE_ALIAS is ugly, you already have a table (usually) of device names that are supported by the driver, the module aliases should be generated from that table. This also keeps supported device list in one place. > As far as I see it will need more code and changes than adding one line of > MODULE_ALIAS. > >> Although it doesn't look like this driver has an SPI id table, you >> should probably add one, I be interested to see if this module is always >> being matched through the "spi" or the "of" alias.. > > Could you please propose how that code should look like, so that I can test? > Sure, start with $ udevadm monitor and see what string the kernel is looking for when trying to find a module for this device. If it is only ever looking for "of:toppoly,td028ttec1", then you can drop the MODULE_ALIAS and be done as it was never getting used anyway. What I expect though is "spi:toppoly,td028ttec1", in which case you should add static const struct spi_device_id td028ttec1_ids[] = { { "toppoly,td028ttec1", 0 }, { "tpo,td028ttec1", 0}, { /* sentinel */ } }; MODULE_DEVICE_TABLE(spi, td028ttec1_ids); link to it in the td028ttec1_spi_driver struct: .id_table = td028ttec1_ids, Then test again to see that the module still loads with the new and old DT string. Andrew > BR and thanks, > Nikolaus Schaller > >> >>> >>> Should I submit a new version? >>> >>> BR, >>> Nikolaus >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe devicetree" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >