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

Groups > linux.kernel > #1714888 > unrolled thread

Re: [RFC v1 4/6] platform: x86: Add generic Intel IPC driver

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 4/6] platform: x86: Add generic Intel IPC driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-18 14:40 +0200
    Re: [RFC v1 4/6] platform: x86: Add generic Intel IPC driver sathya <sathyaosid@gmail.com> - 2017-08-22 07:10 +0200
      Re: [RFC v1 4/6] platform: x86: Add generic Intel IPC driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-22 12:00 +0200

#1714888 — Re: [RFC v1 4/6] platform: x86: Add generic Intel IPC driver

On Tue, Aug 1, 2017 at 9:13 PM,
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c
> redundantly implements the same IPC features and has lot of code
> duplication between them. This driver addresses this issue by grouping
> the common IPC functionalities under the same driver.
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  arch/x86/include/asm/intel_ipc_dev.h | 148 ++++++++++++

No, it should go under include/linux/platform_data/x86/

> +/*
> + * intel_ipc_dev.h: IPC class device header file

No file names in the top of files.

> + *
> + * (C) Copyright 2017 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + *
> + */

> +struct intel_ipc_dev_cfg {
> +       void __iomem *base;
> +       void __iomem *wrbuf_reg;
> +       void __iomem *rbuf_reg;
> +       void __iomem *sptr_reg;
> +       void __iomem *dptr_reg;
> +       void __iomem *status_reg;
> +       void __iomem *cmd_reg;

No, you have to switch to regmap instead.

> +       int mode;
> +       int irq;
> +       int irqflags;
> +       int chan_type;
> +       bool use_msi;
> +};


-- 
With Best Regards,
Andy Shevchenko

[toc] | [next] | [standalone]

#1717022

Hi Andy,


On 08/18/2017 05:38 AM, Andy Shevchenko wrote:
> On Tue, Aug 1, 2017 at 9:13 PM,
> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c
>> redundantly implements the same IPC features and has lot of code
>> duplication between them. This driver addresses this issue by grouping
>> the common IPC functionalities under the same driver.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   arch/x86/include/asm/intel_ipc_dev.h | 148 ++++++++++++
> No, it should go under include/linux/platform_data/x86/
>
>> +/*
>> + * intel_ipc_dev.h: IPC class device header file
> No file names in the top of files.
>
>> + *
>> + * (C) Copyright 2017 Intel Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; version 2
>> + * of the License.
>> + *
>> + */
>> +struct intel_ipc_dev_cfg {
>> +       void __iomem *base;
>> +       void __iomem *wrbuf_reg;
>> +       void __iomem *rbuf_reg;
>> +       void __iomem *sptr_reg;
>> +       void __iomem *dptr_reg;
>> +       void __iomem *status_reg;
>> +       void __iomem *cmd_reg;
> No, you have to switch to regmap instead.
Do you want me to register regmap in intel_pmc_ipc.c and pass this 
regmap pointer to devm_intel_ipc_dev_create()
instead of regular mem address ? Please correct me if my understanding 
is incorrect.

But I don't understand how this change will improve the design.
>
>> +       int mode;
>> +       int irq;
>> +       int irqflags;
>> +       int chan_type;
>> +       bool use_msi;
>> +};
>
-
Sathya

[toc] | [prev] | [next] | [standalone]

#1717236

On Tue, Aug 22, 2017 at 8:09 AM, sathya <sathyaosid@gmail.com> wrote:
> On 08/18/2017 05:38 AM, Andy Shevchenko wrote:
>> On Tue, Aug 1, 2017 at 9:13 PM,
>> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:

>>> +struct intel_ipc_dev_cfg {
>>> +       void __iomem *base;
>>> +       void __iomem *wrbuf_reg;
>>> +       void __iomem *rbuf_reg;
>>> +       void __iomem *sptr_reg;
>>> +       void __iomem *dptr_reg;
>>> +       void __iomem *status_reg;
>>> +       void __iomem *cmd_reg;
>>
>> No, you have to switch to regmap instead.
>
> Do you want me to register regmap in intel_pmc_ipc.c and pass this regmap
> pointer to devm_intel_ipc_dev_create()
> instead of regular mem address ? Please correct me if my understanding is
> incorrect.
>
> But I don't understand how this change will improve the design.

We will have a core part which takes a PMC/SCU regmap on input.

Core part will not know about exact PMC in use. On top of core part
you will have one driver per PMC type.

In exchange core part will provide a generic set of functions like
"send simple command", "send command", "do ...smth...".

Core part _is_ a library.

>>> +       int mode;
>>> +       int irq;
>>> +       int irqflags;
>>> +       int chan_type;
>>> +       bool use_msi;
>>> +};

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web