Re: [PATCH v2 18/41] arm: Introduce a generic way to use a device from acpi

[Parth Dixit <parth.dixit@linaro.org>]

+shannon

On 25 May 2015 at 17:08, Parth Dixit <parth.dixit@linaro.org> wrote:
> On 25 May 2015 at 15:30, Julien Grall <julien.grall@citrix.com> wrote:
>> Hi Parth,
>>
>> On 25/05/2015 07:58, Parth Dixit wrote:
>>>
>>> On 24 May 2015 at 13:10, Julien Grall <julien.grall@citrix.com> wrote:
>>>>
>>>> On 24/05/2015 08:06, Parth Dixit wrote:
>>>>>
>>>>>
>>>>>       > +struct acpi_device_desc {
>>>>>       > +    /* Device name */
>>>>>       > +    const char *name;
>>>>>       > +    /* Device class */
>>>>>       > +    enum device_class class;
>>>>>       > +    /* type of device supported by the driver */
>>>>>       > +    const int class_type;
>>>>>       > +    /* Device initialization */
>>>>>       > +    int (*init)(const void *data);
>>>>>       > +};
>>>>>
>>>>>      Given that the number of device will be minimal in Xen, I would
>>>>> prefer
>>>>>      to merge this structure into device_desc by adding the ACPI fields.
>>>>>
>>>>>      It would avoid to duplicate everything for only 2 fields changes.
>>>>>
>>>>>       From the drivers point of view it would look like
>>>>>
>>>>>      DEVICE_START(....)
>>>>>               .dt_init = ...
>>>>>      #ifdef CONFIG_ACPI
>>>>>               .acpi_init = ...
>>>>>      #endif
>>>>>      DEVICE_END
>>>>>
>>>>>      Or something like
>>>>>
>>>>>      DEVICE_START(...)
>>>>>               DT_INIT(...)
>>>>>               ACPI_INIT(...)
>>>>>      DEVICE_END
>>>>>
>>>>>      And ACPI_INIT will be a no-op when CONFIG_ACPI is not enabled.
>>>>>
>>>>> I think we agreed not to use common structure as it had some dt specific
>>>>> entries and there was scope of confusion.
>>>>
>>>>
>>>>
>>>> I don't remember a such agreement. So far, only compatible and init are
>>>> DT
>>>> specific. The rest (most of the fields) are device agnostic.
>>>
>>> Adding attachment of the previous discussion
>>
>>
>> Thanks. Please a give link to the conversation (such as a mail archive)
>> rather than an attachment. I had to look on the archive to find the context
>> of this conversation...
> ah, sorry about that, i keep forgetting that this conversation is also
> available in public list and i can provide a link to it.
>> Also, that something useful to add in the notes of the patch (after ---).
>>
>> Regards,
>>
>> --
>> Julien Grall