From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v6,01/12] drivers: base: Unified device connection lookup From: Hans de Goede Message-Id: Date: Sun, 11 Mar 2018 19:24:18 +0100 To: Greg Kroah-Hartman Cc: Darren Hart , Andy Shevchenko , MyungJoo Ham , Chanwoo Choi , Mathias Nyman , Heikki Krogerus , Guenter Roeck , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org List-ID: SGkgYWxsLAoKT24gMDktMDMtMTggMTg6NTMsIEdyZWcgS3JvYWgtSGFydG1hbiB3cm90ZToKPiBP biBGcmksIE1hciAwMiwgMjAxOCBhdCAxMToyMDo0NkFNICswMTAwLCBIYW5zIGRlIEdvZWRlIHdy b3RlOgo+PiBGcm9tOiBIZWlra2kgS3JvZ2VydXMgPGhlaWtraS5rcm9nZXJ1c0BsaW51eC5pbnRl bC5jb20+Cj4+Cj4+IFNldmVyYWwgZnJhbWV3b3JrcyAtIGNsaywgZ3BpbywgcGh5LCBwbXcsIGV0 Yy4gLSBtYWludGFpbgo+PiBsb29rdXAgdGFibGVzIGZvciBkZXNjcmliaW5nIGNvbm5lY3Rpb25z IGFuZCBwcm92aWRlIGN1c3RvbQo+PiBBUEkgZm9yIGhhbmRsaW5nIHRoZW0uIFRoaXMgaW50cm9k dWNlcyBhIHNpbmdsZSBnZW5lcmljCj4+IGxvb2t1cCB0YWJsZSBhbmQgQVBJIGZvciB0aGUgY29u bmVjdGlvbnMuCj4+Cj4+IFRoZSBtb3RpdmF0aW9uIGZvciB0aGlzIGNvbW1pdCBpcyBjZW50cmFs aXppbmcgdGhlCj4+IGNvbm5lY3Rpb24gbG9va3VwLCBidXQgdGhlIGdvYWwgaXMgdG8gdWx0aW1h dGVseSBleHRyYWN0IHRoZQo+PiBjb25uZWN0aW9uIGRlc2NyaXB0aW9ucyBhbHNvIGZyb20gZmly bXdhcmUgYnkgdXNpbmcgdGhlCj4+IGZ3bm9kZV9ncmFwaF8qIGZ1bmN0aW9ucyBhbmQgb3RoZXIg bWVjaGFuaXNtcyB0aGF0IGFyZQo+PiBhdmFpbGFibGUuCj4+Cj4+IFNpZ25lZC1vZmYtYnk6IEhl aWtraSBLcm9nZXJ1cyA8aGVpa2tpLmtyb2dlcnVzQGxpbnV4LmludGVsLmNvbT4KPj4gUmV2aWV3 ZWQtYnk6IEhhbnMgZGUgR29lZGUgPGhkZWdvZWRlQHJlZGhhdC5jb20+Cj4+IFJldmlld2VkLWJ5 OiBBbmR5IFNoZXZjaGVua28gPGFuZHkuc2hldmNoZW5rb0BnbWFpbC5jb20+Cj4+IFNpZ25lZC1v ZmYtYnk6IEhhbnMgZGUgR29lZGUgPGhkZWdvZWRlQHJlZGhhdC5jb20+Cj4gCj4gU29ycnkgZm9y IHRoZSBkZWxheSwganVzdCBub3cgcmV2aWV3aW5nIHRoaXMgcGF0Y2guLi4KPiAKPiBUaGUgY29u dGVudCBpcyBmaW5lIChpZiBub3Qgc2NhcnkgZm9yIHRoZSBvYnZpb3VzIHJlYXNvbiBvZiBwYXNz aW5nCj4gYXJvdW5kICdzdHJ1Y3QgZGV2aWNlJyBvZiBkaWZmZXJlbnQgYnVzIHR5cGVzLCBidXQg b2suLi4pLCBidXQgdGhlIGFwaQo+IG5hbWluZyBpcyAicm91Z2giOgoKCkhlaWtraSwgSSB0aGlu ayBpdCBpcyBiZXN0IGlmIHlvdSBhbnN3ZXIgR3JlZydzIHJlbWFya3MuIEZXSVcgSSdtCmZpbmUg d2l0aCB0aGUgY2hhbmdlcyBHcmVnIHByb3Bvc2VzLgoKSSBjdXJyZW50bHkgaGF2ZSBzaWduaWZp Y2FudGx5IGxlc3MgYmFuZHdpZHRoIGZvciB0aGlzIGR1ZSB0bwpwZXJzb25hbCBjaXJjdW1zdGFu Y2VzLCBzbyBpZiBhIG5ldyB2ZXJzaW9uIG9mIHRoaXMgcGF0Y2gtc2V0CmlzIG5lY2Vzc2FyeSBp dCB3b3VsZCBiZSBncmVhdCBpZiB5b3UgKEhlaWtraSkgY2FuIGRvIGEgdjcuCgpSZWdhcmRzLAoK SGFucwoKPiAKPj4gLS0tIC9kZXYvbnVsbAo+PiArKysgYi9pbmNsdWRlL2xpbnV4L2Nvbm5lY3Rp b24uaAo+IAo+ICJjb25uZWN0aW9uLmgiIGlzIHZhZ3VlLCB3aHkgbm90IGp1c3QgcHV0IHRoaXMg aW4gZGV2aWNlLmg/ID4KPj4gQEAgLTAsMCArMSwzNCBAQAo+PiArLy8gU1BEWC1MaWNlbnNlLUlk ZW50aWZpZXI6IEdQTC0yLjAKPj4gKwo+PiArI2lmbmRlZiBfTElOVVhfQ09OTkVDVElPTl9IXwo+ PiArI2RlZmluZSBfTElOVVhfQ09OTkVDVElPTl9IXwo+PiArCj4+ICsjaW5jbHVkZSA8bGludXgv bGlzdC5oPgo+PiArCj4+ICtzdHJ1Y3QgZGV2aWNlOwo+PiArCj4+ICsvKioKPj4gKyAqIHN0cnVj dCBkZXZjb24gLSBEZXZpY2UgQ29ubmVjdGlvbiBEZXNjcmlwdG9yCj4+ICsgKiBAZW5kcG9pbnQ6 IFRoZSBuYW1lcyBvZiB0aGUgdHdvIGRldmljZXMgY29ubmVjdGVkIHRvZ2V0aGVyCj4+ICsgKiBA aWQ6IFVuaXF1ZSBpZGVudGlmaWVyIGZvciB0aGUgY29ubmVjdGlvbgo+PiArICogQGxpc3Q6IExp c3QgaGVhZCwgcHJpdmF0ZSBmb3IgZGV2Y29uIGludGVybmFsIHVzZSBvbmx5Cj4+ICsgKi8KPj4g K3N0cnVjdCBkZXZjb24gewo+IAo+ICdzdHJ1Y3QgZGV2aWNlX2Nvbm5lY3Rpb24nPyAgU3BlbGwg aXQgb3V0IHBsZWFzZSwgcGVvcGxlIG1pZ2h0IHRoaW5rCj4gdGhpcyBpcyBhICJkZXZlbG9wZXIg Y29uZmVyZW5jZSIgOikKPiAKPj4gKwljb25zdCBjaGFyCQkqZW5kcG9pbnRbMl07Cj4+ICsJY29u c3QgY2hhcgkJKmlkOwo+PiArCXN0cnVjdCBsaXN0X2hlYWQJbGlzdDsKPj4gK307Cj4+ICsKPj4g K3ZvaWQgKl9fZGV2aWNlX2ZpbmRfY29ubmVjdGlvbihzdHJ1Y3QgZGV2aWNlICpkZXYsIGNvbnN0 IGNoYXIgKmNvbl9pZCwKPj4gKwkJCSAgICAgICB2b2lkICpkYXRhLAo+PiArCQkJICAgICAgIHZv aWQgKigqbWF0Y2gpKHN0cnVjdCBkZXZjb24gKmNvbiwgaW50IGVwLAo+PiArCQkJCQkgICAgICB2 b2lkICpkYXRhKSk7Cj4gCj4gSWNrLCBfXyogZnVuY3Rpb25zIGFyZSB1c3VhbGx5ICJubyBsb2Nr IG5lZWRlZCIsIGJ1dCBoZXJlIHlvdSBhcmUgZG9pbmcKPiBhIGxvdCAibW9yZSIgdGhhbiB0aGUg bm9ybWFsIGRldmljZV9maW5kX2Nvbm5lY3Rpb24oKSBjYWxsLiAgV2h5IG5vdAo+IG1ha2UgdGhp czoKPiAJZGV2aWNlX2Nvbm5lY3Rpb25fZmluZF9tYXRjaCgpPwo+IAo+PiArCj4+ICtzdHJ1Y3Qg ZGV2aWNlICpkZXZpY2VfZmluZF9jb25uZWN0aW9uKHN0cnVjdCBkZXZpY2UgKmRldiwgY29uc3Qg Y2hhciAqY29uX2lkKTsKPiAKPiBkZXZpY2VfY29ubmVjdGlvbl9maW5kKCk/Cj4gCj4+ICsKPj4g KyNkZWZpbmUgREVWQ09OKF9lcDAsIF9lcDEsIF9pZCkgICAgKHN0cnVjdCBkZXZjb24pIHsgeyBf ZXAwLCBfZXAxIH0sIF9pZCwgfQo+IAo+IENhbiB5b3UgdXNlIG5hbWVkIGlkZW50aWZpZXJzIGhl cmU/Cj4gCj4+ICsKPj4gK3ZvaWQgYWRkX2RldmljZV9jb25uZWN0aW9uKHN0cnVjdCBkZXZjb24g KmNvbik7Cj4+ICt2b2lkIHJlbW92ZV9kZXZpY2VfY29ubmVjdGlvbihzdHJ1Y3QgZGV2Y29uICpj b24pOwo+IAo+IGRldmljZV9jb25uZWN0aW9uX2FkZCgpIGFuZCBkZXZpY2VfY29ubmVjdGlvbl9y ZW1vdmUoKT8KPiAKPiBJIGNhbiBtYWtlIHRoZSBhcGkgbmFtZSBjaGFuZ2VzIGluIGFuIGFkZC1v biBwYXRjaC4KPiAKPiB0aGFua3MsCj4gCj4gZ3JlZyBrLWgKPgotLS0KVG8gdW5zdWJzY3JpYmUg ZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4K dGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBt YWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5o dG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v6 01/12] drivers: base: Unified device connection lookup Date: Sun, 11 Mar 2018 19:24:18 +0100 Message-ID: References: <20180302102057.8917-1-hdegoede@redhat.com> <20180302102057.8917-2-hdegoede@redhat.com> <20180309175347.GA12150@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180309175347.GA12150@kroah.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Greg Kroah-Hartman Cc: Darren Hart , Andy Shevchenko , MyungJoo Ham , Chanwoo Choi , Mathias Nyman , Heikki Krogerus , Guenter Roeck , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org List-Id: platform-driver-x86.vger.kernel.org Hi all, On 09-03-18 18:53, Greg Kroah-Hartman wrote: > On Fri, Mar 02, 2018 at 11:20:46AM +0100, Hans de Goede wrote: >> From: Heikki Krogerus >> >> Several frameworks - clk, gpio, phy, pmw, etc. - maintain >> lookup tables for describing connections and provide custom >> API for handling them. This introduces a single generic >> lookup table and API for the connections. >> >> The motivation for this commit is centralizing the >> connection lookup, but the goal is to ultimately extract the >> connection descriptions also from firmware by using the >> fwnode_graph_* functions and other mechanisms that are >> available. >> >> Signed-off-by: Heikki Krogerus >> Reviewed-by: Hans de Goede >> Reviewed-by: Andy Shevchenko >> Signed-off-by: Hans de Goede > > Sorry for the delay, just now reviewing this patch... > > The content is fine (if not scary for the obvious reason of passing > around 'struct device' of different bus types, but ok...), but the api > naming is "rough": Heikki, I think it is best if you answer Greg's remarks. FWIW I'm fine with the changes Greg proposes. I currently have significantly less bandwidth for this due to personal circumstances, so if a new version of this patch-set is necessary it would be great if you (Heikki) can do a v7. Regards, Hans > >> --- /dev/null >> +++ b/include/linux/connection.h > > "connection.h" is vague, why not just put this in device.h? > >> @@ -0,0 +1,34 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +#ifndef _LINUX_CONNECTION_H_ >> +#define _LINUX_CONNECTION_H_ >> + >> +#include >> + >> +struct device; >> + >> +/** >> + * struct devcon - Device Connection Descriptor >> + * @endpoint: The names of the two devices connected together >> + * @id: Unique identifier for the connection >> + * @list: List head, private for devcon internal use only >> + */ >> +struct devcon { > > 'struct device_connection'? Spell it out please, people might think > this is a "developer conference" :) > >> + const char *endpoint[2]; >> + const char *id; >> + struct list_head list; >> +}; >> + >> +void *__device_find_connection(struct device *dev, const char *con_id, >> + void *data, >> + void *(*match)(struct devcon *con, int ep, >> + void *data)); > > Ick, __* functions are usually "no lock needed", but here you are doing > a lot "more" than the normal device_find_connection() call. Why not > make this: > device_connection_find_match()? > >> + >> +struct device *device_find_connection(struct device *dev, const char *con_id); > > device_connection_find()? > >> + >> +#define DEVCON(_ep0, _ep1, _id) (struct devcon) { { _ep0, _ep1 }, _id, } > > Can you use named identifiers here? > >> + >> +void add_device_connection(struct devcon *con); >> +void remove_device_connection(struct devcon *con); > > device_connection_add() and device_connection_remove()? > > I can make the api name changes in an add-on patch. > > thanks, > > greg k-h >