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). 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. 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 -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
