On 10 November 2014 23:04, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote: > On 11 November 2014 01:43, Mark Brown <broonie@xxxxxxxxxx> 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: >>> > On 10 November 2014 09:16, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote: >>> >> On 10 November 2014 19:23, 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. >> > I don't mean that by 'bad design'. > Ashwin asked what if a client wants the channel from, say, second > instance of PCC instead of first (both are same controllers because > they check for same signature). 2 channels from 2 "different" controllers. e.g. Non-PCC controllers, which don't have a "subspace" signature field like PCC does. > I can not imagine a good reason why would a client want that. If the > reason is like only the channel of first controller is actually > connected to remote (while that of second controller is nipped) ... I > say that kind of board specific knowledge belongs closer to controller > than the client. The second controller should have known that and not > even populated the channel. > > >> 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. >> >> 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. >> > ACPI does specify a PCC specific signature (0x50434300) for its > channels. Any channel is identified by doing OR of that signature and > the index of channel/subspace in the array of 256. This is true only for PCC. Another class of controllers may not even have such a signature. The only thing you can be sure of is that they will have the 4 letter table identifier string e.g. PCCT. > > IF another class of controllers is given the same name "_PCC", the > ACPI will atleast assign it a different signature (if not ACPI, the > controller driver for Linux). So we need not even think about using > strings. > > >> 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. >> > We already have a way to circumvent all of that. I don't just object > to Ashwin's patch, I posted a more generic and robust solution here > https://lkml.org/lkml/2014/11/9/75 Arnd, Mark, Rafael, Lv, please^n ACK or NAK one of these patches and help us move on. > > -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
