On 12 November 2014 14:04, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote:
> 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()?
Yea. Just hit that problem. :)
>
> -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
