On 11 November 2014 01:59, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Monday 10 November 2014 20:13:10 Mark Brown wrote: >> On Mon, Nov 10, 2014 at 11:14:48PM +0530, Jassi Brar wrote: >> > On 10 November 2014 20:17, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote: >> > > What is there to stop two users from coming up with the same signature >> > > (0xdeadbeef / "string") for their mbox controllers? Things can break >> > > at run time, because with your patch, the first mbox controller with >> > > the duplicate signature/name will return a channel. The client may be >> > > expecting a channel from the "other" mbox. >> >> > Two channels with same signature are supposed to be _identical_ i.e, >> > either channel could serve any client asking for a channel with that >> > signature. So even if an "unexpected" instance of the channel is >> > assigned, the client should still be happy. >> >> > If a client differentiates between 2 instances of a channel, that's >> > probably a sign of bad design. The knowledge behind client's >> > preference of instance should actually lie on the provider(controller) >> > side. I am open to some example on the contrary. >> >> The problem here is that ACPI isn't defining the context in a way which >> maps well onto the way Linux looks things up. We may not like that and >> think it's a bad design but the spec is a done deal here and we have to >> address reality. From an ACPI point of view the context is the fact >> that this is a PCC channel (there's a globally unique namespace for PCC >> channels) but Linux wants a struct device for the client to represent >> the context with a mapping table of some kind behind that to do the >> lookup. > > Right. > >> There's nothing in the ACPI description of the channel or controller to >> tie it to the client device, and there's nothing preventing some other >> mailbox mechanism that gets added to an ACPI system from reusing similar >> names (bear in mind that idiomatic naming for ACPI appears to be three >> or four character strings). If we have a PCC channel "FOO" and some new >> mailbox type which also defines "FOO" the controllers aren't really >> going to be able to tell them apart just on the string. >> >> We could fit the maping into Linux a bit by having a struct device >> representing the PCC controller that you use to do the lookup but at >> that point you may as well have a PCC specific request function that >> knows that device and does the lookup which is approximately what we >> have in Ashwin's patch. We could also require that the lookup be >> something like "PCC:FOO" and use a global_xlate() but it's not clear to >> me that this is making things clearer. > > Agreed. Having a special interface here the way that Ashwin's patch > introduces seems the least invasive. > Just to be sure... please have a look at an alternative solution https://lkml.org/lkml/2014/11/9/75 I think 0 new api is lesser invasive than 2 new apis. > I don't think we would risk running > into the problems that Jassi mentioned regarding API growth if we > get other cases. > Do we anticipate .. a) more than one instance of PCC on a platform? b) PCC alongside another non-DT/ACPI mailbox device? I discount (a). But for (b) it doesn't look neat to either add yet another PCC like api or make non-PCC clients call pcc_mbox_request_channel(). Adapting mbox_request_channe() to accommodate even non-DT clients (like PCC) seems cleaner and more future proof. > In particular, anything that boots with DT will be > able to use the standard method, and even with ACPI if we have additional > mailboxes we would most likely use the named properties extension in > the future. > > On a side note, I think we will actually have to add a DT binding > to PCC as well, but that should probably provide both the standard > mailbox API as well as support the pcc specific interfaces in order > to allow client drivers that do not have multiple ways of doing the > same thing. > In that case, wouldn't it be cleaner to have client drivers just call mbox_request_channel() irrespective of being populated by DT or ACPI? Please note, in my implementation the client just need to see if it can populate the mbox_client.dev to be able to work as such on both DT backed and ACPI backed controller driver. -Jassi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
