From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=7rgC=IC=renesas.com=yoshihiro.shimoda.uh@kernel.org>
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Subject: RE: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph
Date: Tue, 15 May 2018 11:02:57 +0000
Message-ID: <OSBPR01MB2293D83364ADD5467D7A20AAD8930@OSBPR01MB2293.jpnprd01.prod.outlook.com>
References: <1526289360-3997-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
 <1526289360-3997-2-git-send-email-yoshihiro.shimoda.uh@renesas.com>
 <20180514132740.GE21435@kuha.fi.intel.com>
 <OSBPR01MB22932EC3210BC8097E0B7F0FD8930@OSBPR01MB2293.jpnprd01.prod.outlook.com>
 <20180515082911.GH21435@kuha.fi.intel.com>
In-Reply-To: <20180515082911.GH21435@kuha.fi.intel.com>
Content-Language: ja-JP
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "mark.rutland@arm.com" <mark.rutland@arm.com>, "hdegoede@redhat.com" <hdegoede@redhat.com>, "andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>, "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
List-ID: <devicetree@vger.kernel.org>

Hi Heikki,

Thank you for the reply!

> From: Heikki Krogerus, Sent: Tuesday, May 15, 2018 5:29 PM
>=20
> On Tue, May 15, 2018 at 02:19:14AM +0000, Yoshihiro Shimoda wrote:
> > Hi Heikki,
> >
> > > From: Heikki Krogerus, Sent: Monday, May 14, 2018 10:28 PM
> > >
> > > On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:
<snip>
> > > > +struct device *device_connection_find_by_graph(struct device *dev,=
 u32 port,
> > > > +					       u32 endpoint)
> > > > +{
> > > > +	struct bus_type *bus;
> > > > +	struct fwnode_handle *remote;
> > > > +	struct device *conn;
> > > > +
> > > > +	remote =3D fwnode_graph_get_remote_node(dev_fwnode(dev), port, en=
dpoint);
> > > > +	if (!remote)
> > > > +		return NULL;
> > > > +
> > > > +	for (bus =3D generic_match_buses[0]; bus; bus++) {
> > > > +		conn =3D bus_find_device(bus, NULL, remote, generic_graph_match)=
;
> > > > +		if (conn)
> > > > +			return conn;
> > > > +	}
> > > > +
> > > > +	/*
> > > > +	 * We only get called if a connection was found, tell the caller =
to
> > > > +	 * wait for the other device to show up.
> > > > +	 */
> > > > +	return ERR_PTR(-EPROBE_DEFER);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);
> > >
> > > Why do we need more API for walking through the graph?
> >
> > I thought there is difficult to find if a device has multiple ports or =
endpoints.
> > So, I'd like to use port and endpoint number for finding a device.
> >
> > > I'm not sure exactly sure what is going on here, I'll try to study
> > > your patches more when I have time, but the approach looks wrong. Tha=
t
> > > function looks like a helper, but just not that useful one.
> > >
> > > We really should be able to use the existing functions. In practice
> > > device_connection_find_match() should eventually parse the graph, the=
n
> > > fallback to build-in connections if no graph is found. Otherwise
> > > parsing graph here is not really useful at all.
> >
> > I think using device_connection_find_match() for finding the graph beco=
mes complicated.
> > The current arguments of the function is the below:
> >
> > void *device_connection_find_match(struct device *dev, const char *con_=
id,
> > 			       void *data,
> > 			       void *(*match)(struct device_connection *con,
> > 					      int ep, void *data))
> >
> > If finding the graph, the following arguments will be not used.
> >  - con_id
> >  - *con and ep of "match" arguments.
> >
> > This is because these arguments are for the build-in connections.
>=20
> No they're not. You do realize that we can build a temporary struct
> device_connection there and then (in stack) to be supplied for the
> ->match callback.

I understood it.

> > So, should I modify the arguments of the function for finding both
> > the graph and built-in connections somehow?
>=20
> I don't see any need for that. We may need to modify struct
> device_connection, but there should not be any need to touch the API.
>=20
> Your plan to use port and endpoint number is wrong, as they are just
> indexes, and therefore not reliable. Luckily it should be completely
> unnecessary to use them.
>=20
> The way I see this working is that we parse all the endpoints the
> caller device has. If we can't take advantage of the con_id for
> identification (though ideally we can), we just need to call ->match
> with every one of them. The implementation for the ->match callback is
> then responsible of figuring out if the endpoint is the one we are
> looking for or not in that case.

I understood it. But, I need to investigate how to find a device.

> The only problem I see with that is that we may not have a reliable
> way to convert the fwnode pointing to the remote-endpoint parent into
> struct device (if one has already been enumerated) in order to get the
> device name and use it as the second endpoint name in struct
> device_connection. To solve that, we could consider for example just
> adding a new member, fwnode pointer perhaps, to the structure. Or
> perhaps use the endpoint members for something else than device names
> when graph is used, and add a member defining the type of match:
> graph, build-in, etc. There are many things we can consider.

I don't understand why adding such new member(s) can solve that.
Anyway, I will investigate this more...

> I don't know if I'm able to explain what I'm after with this, so if
> you like, I can propose something for this myself. Though that will
> have to wait for few weeks. Right now I'm too busy with other stuff.

Thank you for your proposal! However, I'd like to try to investigate
once more while you are too busy.

Best regards,
Yoshihiro Shimoda

>=20
> Thanks,
>=20
> --
> heikki

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: [PATCH/RFC,v3,1/4] base: devcon: add a new API to find the graph
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Message-Id: <OSBPR01MB2293D83364ADD5467D7A20AAD8930@OSBPR01MB2293.jpnprd01.prod.outlook.com>
Date: Tue, 15 May 2018 11:02:57 +0000
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "mark.rutland@arm.com" <mark.rutland@arm.com>, "hdegoede@redhat.com" <hdegoede@redhat.com>, "andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>, "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
List-ID: <linux-usb.vger.kernel.org>

SGkgSGVpa2tpLAoKVGhhbmsgeW91IGZvciB0aGUgcmVwbHkhCgo+IEZyb206IEhlaWtraSBLcm9n
ZXJ1cywgU2VudDogVHVlc2RheSwgTWF5IDE1LCAyMDE4IDU6MjkgUE0KPiAKPiBPbiBUdWUsIE1h
eSAxNSwgMjAxOCBhdCAwMjoxOToxNEFNICswMDAwLCBZb3NoaWhpcm8gU2hpbW9kYSB3cm90ZToK
PiA+IEhpIEhlaWtraSwKPiA+Cj4gPiA+IEZyb206IEhlaWtraSBLcm9nZXJ1cywgU2VudDogTW9u
ZGF5LCBNYXkgMTQsIDIwMTggMTA6MjggUE0KPiA+ID4KPiA+ID4gT24gTW9uLCBNYXkgMTQsIDIw
MTggYXQgMDY6MTU6NTdQTSArMDkwMCwgWW9zaGloaXJvIFNoaW1vZGEgd3JvdGU6CjxzbmlwPgo+
ID4gPiA+ICtzdHJ1Y3QgZGV2aWNlICpkZXZpY2VfY29ubmVjdGlvbl9maW5kX2J5X2dyYXBoKHN0
cnVjdCBkZXZpY2UgKmRldiwgdTMyIHBvcnQsCj4gPiA+ID4gKwkJCQkJICAgICAgIHUzMiBlbmRw
b2ludCkKPiA+ID4gPiArewo+ID4gPiA+ICsJc3RydWN0IGJ1c190eXBlICpidXM7Cj4gPiA+ID4g
KwlzdHJ1Y3QgZndub2RlX2hhbmRsZSAqcmVtb3RlOwo+ID4gPiA+ICsJc3RydWN0IGRldmljZSAq
Y29ubjsKPiA+ID4gPiArCj4gPiA+ID4gKwlyZW1vdGUgPSBmd25vZGVfZ3JhcGhfZ2V0X3JlbW90
ZV9ub2RlKGRldl9md25vZGUoZGV2KSwgcG9ydCwgZW5kcG9pbnQpOwo+ID4gPiA+ICsJaWYgKCFy
ZW1vdGUpCj4gPiA+ID4gKwkJcmV0dXJuIE5VTEw7Cj4gPiA+ID4gKwo+ID4gPiA+ICsJZm9yIChi
dXMgPSBnZW5lcmljX21hdGNoX2J1c2VzWzBdOyBidXM7IGJ1cysrKSB7Cj4gPiA+ID4gKwkJY29u
biA9IGJ1c19maW5kX2RldmljZShidXMsIE5VTEwsIHJlbW90ZSwgZ2VuZXJpY19ncmFwaF9tYXRj
aCk7Cj4gPiA+ID4gKwkJaWYgKGNvbm4pCj4gPiA+ID4gKwkJCXJldHVybiBjb25uOwo+ID4gPiA+
ICsJfQo+ID4gPiA+ICsKPiA+ID4gPiArCS8qCj4gPiA+ID4gKwkgKiBXZSBvbmx5IGdldCBjYWxs
ZWQgaWYgYSBjb25uZWN0aW9uIHdhcyBmb3VuZCwgdGVsbCB0aGUgY2FsbGVyIHRvCj4gPiA+ID4g
KwkgKiB3YWl0IGZvciB0aGUgb3RoZXIgZGV2aWNlIHRvIHNob3cgdXAuCj4gPiA+ID4gKwkgKi8K
PiA+ID4gPiArCXJldHVybiBFUlJfUFRSKC1FUFJPQkVfREVGRVIpOwo+ID4gPiA+ICt9Cj4gPiA+
ID4gK0VYUE9SVF9TWU1CT0xfR1BMKGRldmljZV9jb25uZWN0aW9uX2ZpbmRfYnlfZ3JhcGgpOwo+
ID4gPgo+ID4gPiBXaHkgZG8gd2UgbmVlZCBtb3JlIEFQSSBmb3Igd2Fsa2luZyB0aHJvdWdoIHRo
ZSBncmFwaD8KPiA+Cj4gPiBJIHRob3VnaHQgdGhlcmUgaXMgZGlmZmljdWx0IHRvIGZpbmQgaWYg
YSBkZXZpY2UgaGFzIG11bHRpcGxlIHBvcnRzIG9yIGVuZHBvaW50cy4KPiA+IFNvLCBJJ2QgbGlr
ZSB0byB1c2UgcG9ydCBhbmQgZW5kcG9pbnQgbnVtYmVyIGZvciBmaW5kaW5nIGEgZGV2aWNlLgo+
ID4KPiA+ID4gSSdtIG5vdCBzdXJlIGV4YWN0bHkgc3VyZSB3aGF0IGlzIGdvaW5nIG9uIGhlcmUs
IEknbGwgdHJ5IHRvIHN0dWR5Cj4gPiA+IHlvdXIgcGF0Y2hlcyBtb3JlIHdoZW4gSSBoYXZlIHRp
bWUsIGJ1dCB0aGUgYXBwcm9hY2ggbG9va3Mgd3JvbmcuIFRoYXQKPiA+ID4gZnVuY3Rpb24gbG9v
a3MgbGlrZSBhIGhlbHBlciwgYnV0IGp1c3Qgbm90IHRoYXQgdXNlZnVsIG9uZS4KPiA+ID4KPiA+
ID4gV2UgcmVhbGx5IHNob3VsZCBiZSBhYmxlIHRvIHVzZSB0aGUgZXhpc3RpbmcgZnVuY3Rpb25z
LiBJbiBwcmFjdGljZQo+ID4gPiBkZXZpY2VfY29ubmVjdGlvbl9maW5kX21hdGNoKCkgc2hvdWxk
IGV2ZW50dWFsbHkgcGFyc2UgdGhlIGdyYXBoLCB0aGVuCj4gPiA+IGZhbGxiYWNrIHRvIGJ1aWxk
LWluIGNvbm5lY3Rpb25zIGlmIG5vIGdyYXBoIGlzIGZvdW5kLiBPdGhlcndpc2UKPiA+ID4gcGFy
c2luZyBncmFwaCBoZXJlIGlzIG5vdCByZWFsbHkgdXNlZnVsIGF0IGFsbC4KPiA+Cj4gPiBJIHRo
aW5rIHVzaW5nIGRldmljZV9jb25uZWN0aW9uX2ZpbmRfbWF0Y2goKSBmb3IgZmluZGluZyB0aGUg
Z3JhcGggYmVjb21lcyBjb21wbGljYXRlZC4KPiA+IFRoZSBjdXJyZW50IGFyZ3VtZW50cyBvZiB0
aGUgZnVuY3Rpb24gaXMgdGhlIGJlbG93Ogo+ID4KPiA+IHZvaWQgKmRldmljZV9jb25uZWN0aW9u
X2ZpbmRfbWF0Y2goc3RydWN0IGRldmljZSAqZGV2LCBjb25zdCBjaGFyICpjb25faWQsCj4gPiAJ
CQkgICAgICAgdm9pZCAqZGF0YSwKPiA+IAkJCSAgICAgICB2b2lkICooKm1hdGNoKShzdHJ1Y3Qg
ZGV2aWNlX2Nvbm5lY3Rpb24gKmNvbiwKPiA+IAkJCQkJICAgICAgaW50IGVwLCB2b2lkICpkYXRh
KSkKPiA+Cj4gPiBJZiBmaW5kaW5nIHRoZSBncmFwaCwgdGhlIGZvbGxvd2luZyBhcmd1bWVudHMg
d2lsbCBiZSBub3QgdXNlZC4KPiA+ICAtIGNvbl9pZAo+ID4gIC0gKmNvbiBhbmQgZXAgb2YgIm1h
dGNoIiBhcmd1bWVudHMuCj4gPgo+ID4gVGhpcyBpcyBiZWNhdXNlIHRoZXNlIGFyZ3VtZW50cyBh
cmUgZm9yIHRoZSBidWlsZC1pbiBjb25uZWN0aW9ucy4KPiAKPiBObyB0aGV5J3JlIG5vdC4gWW91
IGRvIHJlYWxpemUgdGhhdCB3ZSBjYW4gYnVpbGQgYSB0ZW1wb3Jhcnkgc3RydWN0Cj4gZGV2aWNl
X2Nvbm5lY3Rpb24gdGhlcmUgYW5kIHRoZW4gKGluIHN0YWNrKSB0byBiZSBzdXBwbGllZCBmb3Ig
dGhlCj4gLT5tYXRjaCBjYWxsYmFjay4KCkkgdW5kZXJzdG9vZCBpdC4KCj4gPiBTbywgc2hvdWxk
IEkgbW9kaWZ5IHRoZSBhcmd1bWVudHMgb2YgdGhlIGZ1bmN0aW9uIGZvciBmaW5kaW5nIGJvdGgK
PiA+IHRoZSBncmFwaCBhbmQgYnVpbHQtaW4gY29ubmVjdGlvbnMgc29tZWhvdz8KPiAKPiBJIGRv
bid0IHNlZSBhbnkgbmVlZCBmb3IgdGhhdC4gV2UgbWF5IG5lZWQgdG8gbW9kaWZ5IHN0cnVjdAo+
IGRldmljZV9jb25uZWN0aW9uLCBidXQgdGhlcmUgc2hvdWxkIG5vdCBiZSBhbnkgbmVlZCB0byB0
b3VjaCB0aGUgQVBJLgo+IAo+IFlvdXIgcGxhbiB0byB1c2UgcG9ydCBhbmQgZW5kcG9pbnQgbnVt
YmVyIGlzIHdyb25nLCBhcyB0aGV5IGFyZSBqdXN0Cj4gaW5kZXhlcywgYW5kIHRoZXJlZm9yZSBu
b3QgcmVsaWFibGUuIEx1Y2tpbHkgaXQgc2hvdWxkIGJlIGNvbXBsZXRlbHkKPiB1bm5lY2Vzc2Fy
eSB0byB1c2UgdGhlbS4KPiAKPiBUaGUgd2F5IEkgc2VlIHRoaXMgd29ya2luZyBpcyB0aGF0IHdl
IHBhcnNlIGFsbCB0aGUgZW5kcG9pbnRzIHRoZQo+IGNhbGxlciBkZXZpY2UgaGFzLiBJZiB3ZSBj
YW4ndCB0YWtlIGFkdmFudGFnZSBvZiB0aGUgY29uX2lkIGZvcgo+IGlkZW50aWZpY2F0aW9uICh0
aG91Z2ggaWRlYWxseSB3ZSBjYW4pLCB3ZSBqdXN0IG5lZWQgdG8gY2FsbCAtPm1hdGNoCj4gd2l0
aCBldmVyeSBvbmUgb2YgdGhlbS4gVGhlIGltcGxlbWVudGF0aW9uIGZvciB0aGUgLT5tYXRjaCBj
YWxsYmFjayBpcwo+IHRoZW4gcmVzcG9uc2libGUgb2YgZmlndXJpbmcgb3V0IGlmIHRoZSBlbmRw
b2ludCBpcyB0aGUgb25lIHdlIGFyZQo+IGxvb2tpbmcgZm9yIG9yIG5vdCBpbiB0aGF0IGNhc2Uu
CgpJIHVuZGVyc3Rvb2QgaXQuIEJ1dCwgSSBuZWVkIHRvIGludmVzdGlnYXRlIGhvdyB0byBmaW5k
IGEgZGV2aWNlLgoKPiBUaGUgb25seSBwcm9ibGVtIEkgc2VlIHdpdGggdGhhdCBpcyB0aGF0IHdl
IG1heSBub3QgaGF2ZSBhIHJlbGlhYmxlCj4gd2F5IHRvIGNvbnZlcnQgdGhlIGZ3bm9kZSBwb2lu
dGluZyB0byB0aGUgcmVtb3RlLWVuZHBvaW50IHBhcmVudCBpbnRvCj4gc3RydWN0IGRldmljZSAo
aWYgb25lIGhhcyBhbHJlYWR5IGJlZW4gZW51bWVyYXRlZCkgaW4gb3JkZXIgdG8gZ2V0IHRoZQo+
IGRldmljZSBuYW1lIGFuZCB1c2UgaXQgYXMgdGhlIHNlY29uZCBlbmRwb2ludCBuYW1lIGluIHN0
cnVjdAo+IGRldmljZV9jb25uZWN0aW9uLiBUbyBzb2x2ZSB0aGF0LCB3ZSBjb3VsZCBjb25zaWRl
ciBmb3IgZXhhbXBsZSBqdXN0Cj4gYWRkaW5nIGEgbmV3IG1lbWJlciwgZndub2RlIHBvaW50ZXIg
cGVyaGFwcywgdG8gdGhlIHN0cnVjdHVyZS4gT3IKPiBwZXJoYXBzIHVzZSB0aGUgZW5kcG9pbnQg
bWVtYmVycyBmb3Igc29tZXRoaW5nIGVsc2UgdGhhbiBkZXZpY2UgbmFtZXMKPiB3aGVuIGdyYXBo
IGlzIHVzZWQsIGFuZCBhZGQgYSBtZW1iZXIgZGVmaW5pbmcgdGhlIHR5cGUgb2YgbWF0Y2g6Cj4g
Z3JhcGgsIGJ1aWxkLWluLCBldGMuIFRoZXJlIGFyZSBtYW55IHRoaW5ncyB3ZSBjYW4gY29uc2lk
ZXIuCgpJIGRvbid0IHVuZGVyc3RhbmQgd2h5IGFkZGluZyBzdWNoIG5ldyBtZW1iZXIocykgY2Fu
IHNvbHZlIHRoYXQuCkFueXdheSwgSSB3aWxsIGludmVzdGlnYXRlIHRoaXMgbW9yZS4uLgoKPiBJ
IGRvbid0IGtub3cgaWYgSSdtIGFibGUgdG8gZXhwbGFpbiB3aGF0IEknbSBhZnRlciB3aXRoIHRo
aXMsIHNvIGlmCj4geW91IGxpa2UsIEkgY2FuIHByb3Bvc2Ugc29tZXRoaW5nIGZvciB0aGlzIG15
c2VsZi4gVGhvdWdoIHRoYXQgd2lsbAo+IGhhdmUgdG8gd2FpdCBmb3IgZmV3IHdlZWtzLiBSaWdo
dCBub3cgSSdtIHRvbyBidXN5IHdpdGggb3RoZXIgc3R1ZmYuCgpUaGFuayB5b3UgZm9yIHlvdXIg
cHJvcG9zYWwhIEhvd2V2ZXIsIEknZCBsaWtlIHRvIHRyeSB0byBpbnZlc3RpZ2F0ZQpvbmNlIG1v
cmUgd2hpbGUgeW91IGFyZSB0b28gYnVzeS4KCkJlc3QgcmVnYXJkcywKWW9zaGloaXJvIFNoaW1v
ZGEKCj4gCj4gVGhhbmtzLAo+IAo+IC0tCj4gaGVpa2tpCi0tLQpUbyB1bnN1YnNjcmliZSBmcm9t
IHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtdXNiIiBpbgp0aGUg
Ym9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1ham9y
ZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwK