From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755039AbbHCTMq (ORCPT ); Mon, 3 Aug 2015 15:12:46 -0400 Received: from lists.s-osg.org ([54.187.51.154]:55827 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754964AbbHCTMp (ORCPT ); Mon, 3 Aug 2015 15:12:45 -0400 Subject: Re: [PATCH 06/27] misc: eeprom: Export I2C module alias information in missing drivers To: Jean Delvare References: <1438273132-20926-1-git-send-email-javier@osg.samsung.com> <1438273132-20926-7-git-send-email-javier@osg.samsung.com> <20150803130550.12487244@endymion.delvare> <55BF75A2.90200@osg.samsung.com> <1438620615.10722.10.camel@chaos.site> From: Javier Martinez Canillas X-Enigmail-Draft-Status: N1110 Cc: linux-kernel@vger.kernel.org Message-ID: <55BFBD27.3050309@osg.samsung.com> Date: Mon, 3 Aug 2015 21:12:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <1438620615.10722.10.camel@chaos.site> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Jean, On 08/03/2015 06:50 PM, Jean Delvare wrote: > Le Monday 03 August 2015 à 16:07 +0200, Javier Martinez Canillas a > écrit : >> Hello Jean, >> >> On 08/03/2015 01:05 PM, Jean Delvare wrote: >>> Hi Javier, >>> >>> On Thu, 30 Jul 2015 18:18:31 +0200, Javier Martinez Canillas wrote: >>>> The I2C core always reports the MODALIAS uevent as "i2c:>>> regardless if the driver was matched using the I2C id_table or the >>>> of_match_table. So the driver needs to export the I2C table and this >>>> be built into the module or udev won't have the necessary information >>>> to auto load the correct module when the device is added. >>>> >>>> Signed-off-by: Javier Martinez Canillas >>>> >>>> --- >>>> >>>> drivers/misc/eeprom/eeprom.c | 1 + >>>> drivers/misc/eeprom/max6875.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/misc/eeprom/eeprom.c b/drivers/misc/eeprom/eeprom.c >>>> index b432873def96..4bb54e1c40a7 100644 >>>> --- a/drivers/misc/eeprom/eeprom.c >>>> +++ b/drivers/misc/eeprom/eeprom.c >>>> @@ -203,6 +203,7 @@ static const struct i2c_device_id eeprom_id[] = { >>>> { "eeprom", 0 }, >>>> { } >>>> }; >>>> +MODULE_DEVICE_TABLE(i2c, eeprom_id); >>>> >>>> static struct i2c_driver eeprom_driver = { >>>> .driver = { >>> >>> I seem to recall this one is missing on purpose. The legacy eeprom >>> driver is deprecated in favor of the at24 driver, so no one should >>> declare "eeprom" i2c devices and thus the module alias is useless. So I >>> would leave the legacy eeprom driver alone. >>> >>> The only feature the at24 driver is missing is device auto-detection as >>> far as I know. Maybe it should be added to ease the transition. Or >>> maybe not, I admit I'm not sure. >>> >> >> It's up to you but since the driver is still in mainline and it has an >> i2c_device_id table, an "eeprom" I2C device can be registered and matched >> by the I2C core but if built as a module, it won't be autoloaded. > > The eeprom driver instantiates its own devices, so auto-loading is not > needed. "eeprom" devices should not be found in device trees, that would > be a bug. Adding an alias is an invitation to introduce such bugs thus > my request to not add such an alias. > OK. >> I'm not familiar with the at24 platform so feel free to discard the patch >> if you think that it is not needed and no one is really using this driver >> (although in that case I think we the driver should just be removed). > > I'm really talking about the at24 eeprom driver > (drivers/misc/eeprom/at24.c) which has nothing to to with the at91 > platform, if that's what you are confusing with. > err, I didn't mention at91 but at24 and that was only because you mentioned it before :-) > Yes, the long-term plan is to get rid of the legacy eeprom driver. But > we need a transition path for users. Either the at24 driver should be > able to instantiate SPD and EDID devices as the eeprom driver does, or > we need a user-space helper to do that kind of detection, so that > consumer scripts such as decode-dimms keep working. The former is a > smaller change, I just hope it won't have any drawback. > I'm in fact not familiar with any Atmel SoC. I just noticed that the I2C core always repots MODALIAS uevents as i2c:name> issue and so wrote a script to find all the drivers that could be affected by this. But I'll just blacklist drivers/misc/eeprom/eeprom.c as a false positive. Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America