From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kay Sievers Subject: Re: [PATCH 1/2] i2c: Add support for device alias names Date: Thu, 01 May 2008 17:51:29 +0200 Message-ID: <1209657089.20889.46.camel@linux.site> References: <20080428113052.6d024bda@hyperion.delvare> <20080428113901.2772e9d9@hyperion.delvare> <1209395245.3666.9.camel@linux.site> <20080428174010.5a40c2c0@hyperion.delvare> <1209399377.3666.30.camel@linux.site> <20080501100409.1b04fdb7@hyperion.delvare> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20080501100409.1b04fdb7-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org Errors-To: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org To: Jean Delvare Cc: linuxppc-dev list , Paul Mundt , Scott Wood , Linux I2C List-Id: linux-i2c@vger.kernel.org T24gVGh1LCAyMDA4LTA1LTAxIGF0IDEwOjA0ICswMjAwLCBKZWFuIERlbHZhcmUgd3JvdGU6Cj4g T24gTW9uLCAyOCBBcHIgMjAwOCAxODoxNjoxNyArMDIwMCwgS2F5IFNpZXZlcnMgd3JvdGU6Cj4g PiBPbiBNb24sIDIwMDgtMDQtMjggYXQgMTc6NDAgKzAyMDAsIEplYW4gRGVsdmFyZSB3cm90ZToK PiA+ID4gV2h5IHdvdWxkIGkyYyBkZXZpY2UgbW9kYWxpYXNlcyBldmVyIGNvbnRhaW4gbXVsdGlw bGUgc3RyaW5ncz8gQSBkZXZpY2UKPiA+ID4gY2FuJ3QgaGF2ZSBtdWx0aXBsZSBuYW1lcywgY2Fu IGl0Pwo+ID4gCj4gPiBMaWtlIEFDUEkvUE5QIGRldmljZXMsIHdoaWNoIGNhbiBoYXZlIHNldmVy YWwgY29tcGF0IGlkJ3MsIHdoaWNoIG1lYW5zCj4gPiB0aGF0IGEgc2luZ2xlIGRldmljZSBjYW4g aGF2ZSAibXVsdGlwbGUgbmFtZXMiOgo+ID4gICAkIGNhdCAvc3lzL2J1cy9wbnAvZGV2aWNlcy8w MDowOS9pZAo+ID4gICBJQk0wMDU3Cj4gPiAgIFBOUDBmMTMKPiAKPiBBaCwgSSBkaWRuJ3Qga25v dyBhYm91dCB0aGlzLiBOb3cgSSdtIGN1cmlvdXMgaG93IGl0IGNhbiB3b3JrLiBEb2VzIGl0Cj4g bWVhbiB0aGF0IHNldmVyYWwgZHJpdmVycyBhdHRlbXB0IHRvIGJpbmQgdG8gdGhpcyBkZXZpY2U/ IAoK77u/VGhleSBhcmUgdXN1YWxseSBhbGwgaWQncyBmb3IgdGhlIHNhbWUgdHlwZSBvZiBkZXZp Y2UsIGFuZCBkb24ndCBkZXNjcmliZQptdWx0aXBsZSBmdW5jdGlvbnMgYXQgdGhlIHNhbWUgdGlt ZS4gSW4gbW9zdCBjYXNlcyB0aGUgdmVuZG9yIGlkJ3MsIGxpa2UKIklCTTAwNTciIGhlcmUsIGRv IG5vdCBtYXRjaCBhbnl0aGluZy4KCj4gPiA+IENhbid0IHdlIGp1c3Qgc3RvcCBoYW5kbGVfbW9k ZGV2dGFibGUoKSBmcm9tIGFkZGluZyBhIHRhaWxpbmcgIioiCj4gPiA+IGF1dG9tYXRpY2FsbHks IGFuZCBqdXN0IGxldCB0aGUgZGV2aWNlIHR5cGVzIHdoaWNoIG5lZWQgaXQsIGFkZCBpdCBvbgo+ ID4gPiB0aGVpciBvd24/Cj4gPiAKPiA+IEZvciBhIGxvdCBzdWJzeXN0ZW1zIGl0J3MgZmluZSB0 byBoYXZlIGl0IGFwcGVuZGVkLCBhcyB0aGVyZSBpcyBhCj4gPiBkZWZpbmVkIGxpc3Qgb2YgaWRl bnRpZmllcnMsIHdoaWNoIG11c3QgYXBwZWFyIGluIHRoZSBzYW1lIG9yZGVyLCBhbmQKPiA+IG5l dyBpZGVudGlmaWVycyBhcmUgYXBwZW5kZWQgdG8gdGhlIGVuZC4gU28gdGhlICIqIiBzdGlsbCBt YXRjaGVzCj4gPiBtb2R1bGVzIHdpdGggcG9zc2libHkgZXh0ZW5kZWQgbW9kYWxpYXMgc3RyaW5n cy4KPiAKPiBJIHVuZGVyc3RhbmQgdGhlIGxvZ2ljLCBob3dldmVyIEkgYW0gc2tlcHRpY2FsIGhv dyB1c2VmdWwgaXQgaXMgaW4KPiBwcmFjdGljZS4gSWYgd2UgYWRkIGFuIGlkZW50aWZpZXIgdG8g dGhlIGRldmljZSBhbGlhc2VzLCB0aGVuIHdlIGFsc28KPiB1cGRhdGUgdGhlIGNvcnJlc3BvbmRp bmcgbW9kYWxpYXMsIHNvIG5vIGluLXRyZWUgZHJpdmVyIGNhbiBicmVhay4gVGhlCj4gb25seSBj YXNlIHdoZXJlIGl0IG1ha2VzIGEgZGlmZmVyZW5jZSwgYXMgZmFyIGFzIEkgY2FuIHNlZSwgaXMg Zm9yCj4gb3V0LW9mLXRyZWUgZHJpdmVycy4gQW0gSSBjb3JyZWN0PwoK77u/VGhhdCBzb3VuZHMg Y29ycmVjdCwgeWVzLgoKPiBPbiB0b3Agb2YgdGhhdCwgSSBkb3VidCB0aGF0IHdlCj4gYWN0dWFs bHkgYWRkIG5ldyBpZGVudGlmaWVycyB0aGF0IGZyZXF1ZW50bHksIGRvIHdlPwoKTm90IHRoYXQg SSBrbm93LgoKPiA+IFdlIHdvdWxkIGFsc28gbmVlZCB0byByZXZpZXcgYWxsIGJ1c2VzIHdoaWNo IGV4cG9ydCBtb2RhbGlhcywgaWYgdGhleQo+ID4gbmVlZCB0aGUgIioiIG9yIG5vdCwgYW5kIGFk ZCB0aGVtIGJ5IGhhbmQsIGlmIG5lZWRlZC4KPiA+IAo+ID4gSSBndWVzcywgaXQncyBlYXNpZXIg dG8gaW50cm9kdWNlIGFuIGFkZGl0aW9uYWwgcGFyYW1ldGVyIHRvCj4gPiBmaWxlMmFsaWFzOjpk b190YWJsZSgpIGFuZCBzdXBwcmVzcyB0aGUgdHJhaWxpbmcgIioiIGZvciBpMmM/Cj4gCj4gVGhh dCdzIG9uZSBwb3NzaWJpbGl0eSwgYnV0IEkgaGFkIGEgc2xpZ2h0bHkgZGlmZmVyZW50IGFwcHJv YWNoLCB3aGljaAo+IGlzIHRvIGp1c3QgbGV0IHRoZSB0eXBlLXNwZWNpZmljIGhhbmRsZXJzIGFk ZCB0aGUgdHJhaWxpbmcgIioiIGJ5Cj4gdGhlbXNlbHZlcyBpZiB0aGV5IG5lZWQgaXQuIFRoaXMg YWxsb3dzIGZvciBvcHRpbWl6YXRpb24gaW4gYSBmZXcKPiBjYXNlcy4KPiAKPiBTdWJqZWN0OiBt b2Rwb3N0OiBpMmMgYWxpYXNlcyBuZWVkIG5vIHRyYWlsaW5nIHdpbGRjYXJkCj4gCj4gTm90IGFs bCBkZXZpY2UgdHlwZSBhbGlhc2VzIG5lZWQgYSB0cmFpbGluZyB3aWxkY2FyZCwgaW4gcGFydGlj dWxhcgo+IHRoZSBpMmMgYWxpYXNlcyBkb24ndC4gRG9uJ3QgYWRkIGEgd2lsZGNhcmQgYnkgZGVm YXVsdCBpbiBkb190YWJsZSgpLAo+IGluc3RlYWQgbGV0IGVhY2ggZGV2aWNlIHR5cGUgaGFuZGxl ciBhZGQgaXQgaWYgbmVlZGVkLgoKLi4uCgo+IFRoZSBwYXRjaCBvbmx5IGNoYW5nZXMgdGhlIGky YyBhbGlhc2VzLCBhbGwgdGhlIHJlc3QgaXMgdGhlIHNhbWUgYXMKPiBiZWZvcmUgKHVubGVzcyBJ IG1lc3NlZCB1cCBzb21ld2hlcmUsIHRoYXQgaXMuKSBEbyB5b3UgdGhpbmsgdGhpcyB3b3VsZAo+ IGJlIGFjY2VwdGFibGUgZm9yIHVwc3RyZWFtPyBJZiB5b3UgdGhpbmsgaXQncyBiZXR0ZXIgdG8g YWRkIGEgcGFyYW1ldGVyCj4gdG8gZG9fdGFibGUoKSBhbmQgbGV0IGl0IGFkZCB0aGUgIioiIGFz IGl0IGRpZCBzbyBmYXIsIHRoYXQncyBhbHNvIGZpbmUKPiB3aXRoIG1lLCBJIGNhbiB1cGRhdGUg dGhlIHBhdGNoIHRvIGRvIHRoYXQuCgpMb29rcyBmaW5lIHRvIG1lLgoKSWYgdGhlIGNvbnRlbnQg b2Y6CiAgL2xpYi9tb2R1bGVzLyQodW5hbWUgLXIpL21vZHVsZXMuYWxpYXMKbG9va3MgY29ycmVj dCBhZnRlciB0aGUgcGF0Y2gsIHRoaXMgc2hvdWxkIGdvIGluLCBJIHRoaW5rLgoKVGhhbmtzLApL YXkKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwppMmMg bWFpbGluZyBsaXN0CmkyY0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5v cmcvbWFpbG1hbi9saXN0aW5mby9pMmM= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.174]) by ozlabs.org (Postfix) with ESMTP id 3A986DDEC6 for ; Fri, 2 May 2008 01:53:03 +1000 (EST) Subject: Re: [PATCH 1/2] i2c: Add support for device alias names From: Kay Sievers To: Jean Delvare In-Reply-To: <20080501100409.1b04fdb7@hyperion.delvare> References: <20080428113052.6d024bda@hyperion.delvare> <20080428113901.2772e9d9@hyperion.delvare> <1209395245.3666.9.camel@linux.site> <20080428174010.5a40c2c0@hyperion.delvare> <1209399377.3666.30.camel@linux.site> <20080501100409.1b04fdb7@hyperion.delvare> Content-Type: text/plain; charset=utf-8 Date: Thu, 01 May 2008 17:51:29 +0200 Message-Id: <1209657089.20889.46.camel@linux.site> Mime-Version: 1.0 Cc: linuxppc-dev list , Paul Mundt , Scott Wood , Linux I2C List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Thu, 2008-05-01 at 10:04 +0200, Jean Delvare wrote: > On Mon, 28 Apr 2008 18:16:17 +0200, Kay Sievers wrote: > > On Mon, 2008-04-28 at 17:40 +0200, Jean Delvare wrote: > > > Why would i2c device modaliases ever contain multiple strings? A devi= ce > > > can't have multiple names, can it? > >=20 > > Like ACPI/PNP devices, which can have several compat id's, which means > > that a single device can have "multiple names": > > $ cat /sys/bus/pnp/devices/00:09/id > > IBM0057 > > PNP0f13 >=20 > Ah, I didn't know about this. Now I'm curious how it can work. Does it > mean that several drivers attempt to bind to this device?=20 =EF=BB=BFThey are usually all id's for the same type of device, and don't d= escribe multiple functions at the same time. In most cases the vendor id's, like "IBM0057" here, do not match anything. > > > Can't we just stop handle_moddevtable() from adding a tailing "*" > > > automatically, and just let the device types which need it, add it on > > > their own? > >=20 > > For a lot subsystems it's fine to have it appended, as there is a > > defined list of identifiers, which must appear in the same order, and > > new identifiers are appended to the end. So the "*" still matches > > modules with possibly extended modalias strings. >=20 > I understand the logic, however I am skeptical how useful it is in > practice. If we add an identifier to the device aliases, then we also > update the corresponding modalias, so no in-tree driver can break. The > only case where it makes a difference, as far as I can see, is for > out-of-tree drivers. Am I correct? =EF=BB=BFThat sounds correct, yes. > On top of that, I doubt that we > actually add new identifiers that frequently, do we? Not that I know. > > We would also need to review all buses which export modalias, if they > > need the "*" or not, and add them by hand, if needed. > >=20 > > I guess, it's easier to introduce an additional parameter to > > file2alias::do_table() and suppress the trailing "*" for i2c? >=20 > That's one possibility, but I had a slightly different approach, which > is to just let the type-specific handlers add the trailing "*" by > themselves if they need it. This allows for optimization in a few > cases. >=20 > Subject: modpost: i2c aliases need no trailing wildcard >=20 > Not all device type aliases need a trailing wildcard, in particular > the i2c aliases don't. Don't add a wildcard by default in do_table(), > instead let each device type handler add it if needed. ... > The patch only changes the i2c aliases, all the rest is the same as > before (unless I messed up somewhere, that is.) Do you think this would > be acceptable for upstream? If you think it's better to add a parameter > to do_table() and let it add the "*" as it did so far, that's also fine > with me, I can update the patch to do that. Looks fine to me. If the content of: /lib/modules/$(uname -r)/modules.alias looks correct after the patch, this should go in, I think. Thanks, Kay