Re: [Linaro-acpi] [PATCH v10 1/1] Mailbox: Add support for Platform Communication Channel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12 November 2014 23:55, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote:
> On 11 November 2014 15:25, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> On Tuesday 11 November 2014 15:01:28 Sudeep Holla wrote:
>>> On 11/11/14 13:18, Ashwin Chaugule wrote:
>>> > On 11 November 2014 05:30, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>>> >> On 07/11/14 14:52, Ashwin Chaugule wrote:
>>> >>> +static struct mbox_chan pcc_mbox_chan[MAX_PCC_SUBSPACES];
>>> >>
>>> >>
>>> >> Really, do you want to allocate 256 structures of mbox_chan even on
>>> >> systems with just 1 - 2 channels(typically that would be the case) ?
>>> >>
>>> >
>>> > Spec says, max of 256. Easier to support it this way.
>>> >
>>>
>>> I tend to disagree, you can allocate dynamically. And use exactly same
>>> logic you use in get_subspace_id to populate con_priv later. When you
>>> parse PCC Subspace, you can just validate header and not record them in
>>> pcc_mbox_chan. You can get the exact count in acpi_table_parse_entries
>>> and then do the required allocation.
>>
>> Right, I think that would be best.
>
> Discussed this with Sudeep on IRC. The parse_pcc_subspace() function
> actually is a useful place to create new mbox channel entries. We get
> a pointer to the pcct_ss (pcc subspace subtable) in it, for each entry
> as it is discovered in the PCCT. If we ignore that pointer, then we
> will need to re-traverse the PCCT in the probe() function, just to get
> those pointers again.
>
> In theory, we could re-traverse the PCCT in the probe function without
> using the API provided by the ACPI framework, but that sounds dicey.
> In the end we would be doing this only to maintain the clever-ish ptr
> math as it is in get_subspace_id(). Alternately, we could replace the
> array with a list.
>
> e.g.
>
> struct pcc_channel {
>   struct list_head node;
>   struct mbox_chan chan;
> };
>
> Then use list_for_each_entry() to get the subspace id from a struct
> mbox_chan* , and list_for_each() + list_entry() to get the channel
> from a subspace index. These lists are not expected to get long in
> practice so traversing them for lookups is not really expensive and
> its only done at init.
>
> Does this make sense?
>
But what array of channels do you pass to mbox_controller_register()?

-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




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux