On Friday 20 June 2014 17:43:05 Ashwin Chaugule wrote:
> >
> > Note that "device" here doesn't have to mean a platform device that
> > is instantiated from DSDT, it can be any mailbox provider that is
> > registered in an arbitrary way, as long as you have a method to map
> > back from the (consumer-device, name-string) tuple back to the
> > (provider, channel) tuple. I have read your patch again now and noticed
> > that you actually tried to do this, but unfortunately you got it
> > wrong by requiring the consumer to fill out the name of the provider
> > in the request. You can't do that, because it's not generic enough
> > to support devices that can be reused, and it means that drivers
> > using the API are never portable between DT and ACPI. You have to
> > get rid of the "ctrl_name" field in the mbox_client structure and
> > change the lookup to be based only on cd->dev and cl->chan_name,
> > using whatever tables you have available in ACPI.
>
> I think you looked at the previous version of the patch. I'm attaching
> the latest version here again FWIW. In this version, I removed the
> "ctrl_name" field and rely on the cl->chan_name to provide the info as
> described in Jassi' original patch.
>
>
> linux/mailbox_client.h
>
> 18 * struct mbox_client - User of a mailbox
> 19 * @dev: The client device
> 20 * @chan_name: The "controller:channel" this client wants
>
> Instead of dev, I added a name string to the mbox controller
> structure. So now the client gets its channel by requesting
> "controller:channel" where controller should match with mbox->name and
> channel becomes an index into mbox->chans[].
>
Right, I looked at the wrong version, sorry about that.
However, it seems you still make the same mistake here: The name that
gets passed as chan_name in the mailbox API is a local identifier
that is supposed to be interpreted for the client device and used
to look up a pointer to the mailbox device and channel. If you require
drivers to put global data (e.g. the mbox->name, or the channel
number) in there, it's impossible to write a driver that works on
both DT and ACPI. If you want to use the mbox_request_channel()
interface from a driver, you need some form of lookup table in
the ACPI data to do the conversion.
The alternative would be not to use mbox_request_channel() at all
for now, but to add a new interface that can only be used PCC and
that matches by ID but is independent of the use of ACPI or DT,
something like:
struct mbox_chan *pcc_mbox_get_channel(struct mbox_client *cl,
char *name, unsigned chan_id,
struct mbox_chan **chan)
{
struct mbox_controller *mbox;
mbox = mbox_find_pcc_controller(name, ...);
*chan = &mbox->chans[chan_id];
return init_channel(*chan, cl);
}
This would mean that we'd have to special-case "pcc" users, which is
not very nice, but at least it would work on both DT and ACPI,
and a future ACPI version could still add support for the mailbox
API later.
Arnd
--
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
