Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714885 > unrolled thread

Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to  create dependent devices Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-18 14:40 +0200
    Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to  create dependent devices sathya <sathyaosid@gmail.com> - 2017-08-22 07:10 +0200

#1714885 — Re: [RFC v1 3/6] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices

On Tue, Aug 1, 2017 at 9:13 PM,
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:

> Currently, we have lot of repetitive code in dependent device resource
> allocation and device creation handling code. This logic can be improved if
> we use MFD framework for dependent device creation. This patch adds this
> support.

> +static int ipc_create_pmc_devices(struct platform_device *pdev)
>  {
> -       int ret;
> -
> -       /* If we have ACPI based watchdog use that instead */
> +       u8 n = 0;
> +       struct mfd_cell *pmc_mfd_cells;
> +
> +       pmc_mfd_cells = devm_kzalloc(&pdev->dev,
> +                       (sizeof(*pmc_mfd_cells) * PMC_IPC_MAX_MFD_BLOCK),
> +                       GFP_KERNEL);
> +       if (!pmc_mfd_cells)
> +               return -ENOMEM;
> +
> +       /* Create PUNIT IPC MFD cell */
> +       pmc_mfd_cells[n].name = PUNIT_DEVICE_NAME;
> +       pmc_mfd_cells[n].id = -1;
> +       pmc_mfd_cells[n].num_resources = ARRAY_SIZE(punit_ipc_resources);
> +       pmc_mfd_cells[n].resources = punit_ipc_resources;
> +       pmc_mfd_cells[n].ignore_resource_conflicts = 1;

Please, use static variables instead of allocated on a heap.

> +       n++;
> +
> +       /* If we have ACPI based watchdog use that instead, othewise create
> +        * a MFD cell for iTCO watchdog*/
>         if (!acpi_has_watchdog()) {

> +               pmc_mfd_cells[n].name = TCO_DEVICE_NAME;
> +               pmc_mfd_cells[n].id = -1;
> +               pmc_mfd_cells[n].platform_data = &tco_info;
> +               pmc_mfd_cells[n].pdata_size = sizeof(tco_info);
> +               pmc_mfd_cells[n].num_resources =
> +                       ARRAY_SIZE(watchdog_ipc_resources);
> +               pmc_mfd_cells[n].resources = watchdog_ipc_resources;
> +               pmc_mfd_cells[n].ignore_resource_conflicts = 1;
> +               n++;
>         }

...and here you do mfd_add_devices() instead of this stuff.

Check how lpc_ich.c designed.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [next] | [standalone]

#1717021

Hi Andy,


On 08/18/2017 05:29 AM, Andy Shevchenko wrote:
> On Tue, Aug 1, 2017 at 9:13 PM,
> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
>> Currently, we have lot of repetitive code in dependent device resource
>> allocation and device creation handling code. This logic can be improved if
>> we use MFD framework for dependent device creation. This patch adds this
>> support.
>> +static int ipc_create_pmc_devices(struct platform_device *pdev)
>>   {
>> -       int ret;
>> -
>> -       /* If we have ACPI based watchdog use that instead */
>> +       u8 n = 0;
>> +       struct mfd_cell *pmc_mfd_cells;
>> +
>> +       pmc_mfd_cells = devm_kzalloc(&pdev->dev,
>> +                       (sizeof(*pmc_mfd_cells) * PMC_IPC_MAX_MFD_BLOCK),
>> +                       GFP_KERNEL);
>> +       if (!pmc_mfd_cells)
>> +               return -ENOMEM;
>> +
>> +       /* Create PUNIT IPC MFD cell */
>> +       pmc_mfd_cells[n].name = PUNIT_DEVICE_NAME;
>> +       pmc_mfd_cells[n].id = -1;
>> +       pmc_mfd_cells[n].num_resources = ARRAY_SIZE(punit_ipc_resources);
>> +       pmc_mfd_cells[n].resources = punit_ipc_resources;
>> +       pmc_mfd_cells[n].ignore_resource_conflicts = 1;
> Please, use static variables instead of allocated on a heap.
I will follow the model used in lpc_ich.c. Will fix it in next version.
>
>> +       n++;
>> +
>> +       /* If we have ACPI based watchdog use that instead, othewise create
>> +        * a MFD cell for iTCO watchdog*/
>>          if (!acpi_has_watchdog()) {
>> +               pmc_mfd_cells[n].name = TCO_DEVICE_NAME;
>> +               pmc_mfd_cells[n].id = -1;
>> +               pmc_mfd_cells[n].platform_data = &tco_info;
>> +               pmc_mfd_cells[n].pdata_size = sizeof(tco_info);
>> +               pmc_mfd_cells[n].num_resources =
>> +                       ARRAY_SIZE(watchdog_ipc_resources);
>> +               pmc_mfd_cells[n].resources = watchdog_ipc_resources;
>> +               pmc_mfd_cells[n].ignore_resource_conflicts = 1;
>> +               n++;
>>          }
> ...and here you do mfd_add_devices() instead of this stuff.
>
> Check how lpc_ich.c designed.
Will fix it in next version.
>

-
Sathya

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web