On 9 November 2014 20:50, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote:
> Hi Jassi,
>
> On 9 November 2014 08:48, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote:
>> On 7 November 2014 20:22, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote:
>>> +
>>> +/**
>>> + * pcc_mbox_request_channel - PCC clients call this function to
>>> + * request a pointer to their PCC subspace, from which they
>>> + * can get the details of communicating with the remote.
>>> + * @cl: Pointer to Mailbox client, so we know where to bind the
>>> + * Channel.
>>> + * @index: The PCC Subspace index as parsed in the PCC client
>>> + * ACPI package. This is used to lookup the array of PCC
>>> + * subspaces as parsed by the PCC Mailbox controller.
>>> + *
>>> + * Return: Pointer to the Mailbox Channel if successful or
>>> + * ERR_PTR.
>>> + */
>>> +struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl,
>>> + int index)
>>> +{
>>> + struct device *dev = pcc_mbox_ctrl.dev;
>>> + struct mbox_chan *chan;
>>> + unsigned long flags;
>>> +
>>> + /*
>>> + * Each PCC Subspace is a Mailbox Channel.
>>> + * The PCC Clients get their PCC Subspace ID
>>> + * from their own tables and pass it here.
>>> + * This returns a pointer to the PCC subspace
>>> + * for the Client to operate on.
>>> + */
>>> + chan = &pcc_mbox_chan[index];
>>> +
>>> + if (!chan || chan->cl) {
>>> + dev_err(dev, "%s: PCC mailbox not free\n", __func__);
>>> + return ERR_PTR(-EBUSY);
>>> + }
>>> +
>>> + spin_lock_irqsave(&chan->lock, flags);
>>> + chan->msg_free = 0;
>>> + chan->msg_count = 0;
>>> + chan->active_req = NULL;
>>> + chan->cl = cl;
>>> + init_completion(&chan->tx_complete);
>>> +
>>> + if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>>> + chan->txdone_method |= TXDONE_BY_ACK;
>>> +
>>> + spin_unlock_irqrestore(&chan->lock, flags);
>>> +
>>> + return chan;
>>> +}
>>> +EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
>>
>> I have just sent out a patch https://lkml.org/lkml/2014/11/9/75 and
>> kept same people in CC.
>> That patch keeps the api same for DT and ACPI drivers.
>
> Thanks for taking a look.
>
> Your patch looks similar in concept to one of my earlier patches. But
>
Using ifdefs wasn't the right way to enable ACPI channel mapping. The
mailbox core should be able to work with both DT and ACPI at the same
time.
> based on the discussions that followed since, we decided that its best
> to add a separate PCC lookup and registration API. The main reason
> being, we dont have a way to list all mbox providers in ACPI in a way
> that DT does. e.g. in DT, the client->dev is used to look up mbox
> controllers. In ACPI, a client cant specify which mbox controllers to
> associate with, if it can attach to multiple. With the PCC specific
> API, if the client calls it, the controller knows where to look,
> because that lookup is PCC specific.
>
> In your patch, the assumption that PCC is the only ACPI mbox provider,
> maybe true today, but that can change anytime.
>
Please read my patch again, we can have ACPI as well as DT populated
clients. All that you intend to do with this patch can be done there
and _without_ adding new apis.
>>> +
>>> + pcc_pdev = platform_create_bundle(&pcc_mbox_driver,
>>> + pcc_mbox_probe, NULL, 0, NULL, 0);
>>> +
>>> + if (!pcc_pdev) {
>>> + pr_err("Err creating PCC platform bundle\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +device_initcall(pcc_init);
>>>
>> IMO the PCC platform_device should be populated by ACPI or DT core.
>> This PCC controller driver should parse the PCCT ... so it populates
>> .global_xlate() which expect the 'global_id' of channel requested to
>> be (0x50434300 | Client_Class_Code) as specified by ACPI.
>>
>> It should be possible to have same client and controller driver work
>> for both ACPI and DT.
>
> Any reason why this patch won't work with DT as well? (with the
> requisite DT bindings)
>
A PCC client would need to call pcc_mbox_request_channel() if
populated by ACPI, and mbox_request_channel() if by DT. We want any
client driver to call just mbox_request_channel()
-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
