diff mbox

[V2,1/3] dma: add Qualcomm Technologies HIDMA management driver

Message ID 1446444460-21600-2-git-send-email-okaya@codeaurora.org
State Superseded, archived
Headers show

Commit Message

Sinan Kaya Nov. 2, 2015, 6:07 a.m. UTC
The Qualcomm Technologies HIDMA device has been designed
to support virtualization technology. The driver has been
divided into two to follow the hardware design. The management
driver is executed in hypervisor context and is the main
management entity for all channels provided by the device.
The channel driver is executed in the hypervisor/guest OS
context.

All channel devices get probed in the hypervisor
context during power up. They show up as DMA engine
channels. Then, before starting the virtualization; each
channel device is unbound from the hypervisor by VFIO
and assigned to the guest machine for control.

This management driver will be used by the system
admin to monitor/reset the execution state of the DMA
channels. This will be the management interface.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 .../devicetree/bindings/dma/qcom_hidma_mgmt.txt    |  56 ++
 drivers/dma/Kconfig                                |  11 +
 drivers/dma/Makefile                               |   1 +
 drivers/dma/qcom_hidma_mgmt.c                      | 747 +++++++++++++++++++++
 4 files changed, 815 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
 create mode 100644 drivers/dma/qcom_hidma_mgmt.c

Comments

Rob Herring Nov. 2, 2015, 3:57 p.m. UTC | #1
On Mon, Nov 2, 2015 at 12:07 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> The Qualcomm Technologies HIDMA device has been designed
> to support virtualization technology. The driver has been
> divided into two to follow the hardware design. The management
> driver is executed in hypervisor context and is the main
> management entity for all channels provided by the device.
> The channel driver is executed in the hypervisor/guest OS
> context.
>
> All channel devices get probed in the hypervisor
> context during power up. They show up as DMA engine
> channels. Then, before starting the virtualization; each
> channel device is unbound from the hypervisor by VFIO
> and assigned to the guest machine for control.
>
> This management driver will be used by the system
> admin to monitor/reset the execution state of the DMA
> channels. This will be the management interface.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  .../devicetree/bindings/dma/qcom_hidma_mgmt.txt    |  56 ++
>  drivers/dma/Kconfig                                |  11 +
>  drivers/dma/Makefile                               |   1 +
>  drivers/dma/qcom_hidma_mgmt.c                      | 747 +++++++++++++++++++++
>  4 files changed, 815 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
>  create mode 100644 drivers/dma/qcom_hidma_mgmt.c
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> new file mode 100644
> index 0000000..514d37d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> @@ -0,0 +1,56 @@
> +Qualcomm Technologies HIDMA Management interface
> +
> +The Qualcomm Technologies HIDMA device has been designed
> +to support virtualization technology. The driver has been
> +divided into two to follow the hardware design. The management
> +driver is executed in hypervisor context and is the main
> +management entity for all channels provided by the device.
> +The channel driver is executed in the hypervisor/guest OS
> +context.

This doesn't really explain what the block is.

> +All channel devices get probed in the hypervisor
> +context during power up. They show up as DMA engine
> +DMA channels. Then, before starting the virtualization; each
> +channel device is unbound from the hypervisor by VFIO
> +and assign to the guest machine for control.
> +
> +This management driver will  be used by the system
> +admin to monitor/reset the execution state of the DMA
> +channels. This will be the management interface.
> +
> +
> +Required properties:
> +- compatible: must contain "qcom,hidma-mgmt"

Please make this more specific. It doesn't match your example either.
Unless "1.0" type versioning is tightly controlled and defined,
version numbers are usually not a good practice.

> +- reg: Address range for DMA device
> +- dma-channels: Number of channels supported by this DMA controller.
> +- max-write-burst-bytes: Maximum write burst in bytes. A memcpy requested is
> +  fragmented to multiples of this amount.
> +- max-read-burst-bytes: Maximum read burst in bytes. A memcpy request is
> +  fragmented to multiples of this amount.
> +- max-write-transactions: Maximum write transactions to perform in a burst
> +- max-read-transactions: Maximum read transactions to perform in a burst

This would be a function of burst-bytes and bus width. Are you sure
you don't me number of outstanding transactions which is a common
parameter for AXI bus peripherals.

> +- channel-reset-timeout: Channel reset timeout for this SOC.

Please add units to property name.

> +- channel-priority: Priority of the channel.
> +  Each dma channel share the same HW bandwidth with other dma channels.
> +  If two requests reach to the HW at the same time from a low priority and
> +  high priority channel, high priority channel will claim the bus.
> +  0=low priority, 1=high priority
> +- channel-weight: Round robin weight of the channel
> +  Since there are only two priority levels supported, scheduling among
> +  the equal priority channels is done via weights.
> +
> +Example:
> +
> +       hidma-mgmt@f9984000 = {
> +               compatible = "qcom,hidma-mgmt-1.0";
> +               reg = <0xf9984000 0x15000>;
> +               dma-channels = 6;
> +               max-write-burst-bytes = 1024;
> +               max-read-burst-bytes = 1024;
> +               max-write-transactions = 31;
> +               max-read-transactions = 31;
> +               channel-reset-timeout = 0x500;
> +               channel-priority = < 1 1 0 0 0 0>;
> +               channel-weight = < 1 13 10 3 4 5>;
> +       };
> +
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sinan Kaya Nov. 2, 2015, 4:20 p.m. UTC | #2
On 11/2/2015 10:57 AM, Rob Herring wrote:
> Please make this more specific. It doesn't match your example either.
> Unless "1.0" type versioning is tightly controlled and defined,
> version numbers are usually not a good practice.
Is there a good example I can look or a wiki about the device-tree 
naming conventions?

I'm more of an ACPI person than DTS.
Timur Tabi Nov. 2, 2015, 5:26 p.m. UTC | #3
On 11/02/2015 10:20 AM, Sinan Kaya wrote:
>>
> Is there a good example I can look or a wiki about the device-tree
> naming conventions?
>
> I'm more of an ACPI person than DTS.

I think Rob is talking about something like this:

	compatible="qcom,hidma-mgmt-1.0", "qcom,hidma-mgmt"

This specifies that this is the v1.0 of the HIDMA management engine (or, 
the management engine for the 1.0 HIDMA device).  That way, if in the 
future there's a v1.1, you can do this:

	compatible="qcom,hidma-mgmt-1.1", "qcom,hidma-mgmt"

The driver will probe only on ""qcom,hidma-mgmt", but in the probe 
function, it can query the version number and act accordingly.

Alternatively, the driver can probe on both:

static const struct of_device_id hidma_match[] = {
	{ .compatible = "qcom,hidma-mgmt-1.0", &v10_struct},
	{ .compatible = "qcom,hidma-mgmt-1.1", &v11_struct},
	{},
};
MODULE_DEVICE_TABLE(of, hidma_match);

And then the probe function will automatically get a pointer to either 
v10_struct or v11_struct.
Rob Herring Nov. 2, 2015, 5:42 p.m. UTC | #4
On Mon, Nov 2, 2015 at 11:26 AM, Timur Tabi <timur@codeaurora.org> wrote:
> On 11/02/2015 10:20 AM, Sinan Kaya wrote:
>>>
>>>
>> Is there a good example I can look or a wiki about the device-tree
>> naming conventions?

There are many examples. Generally, it is the form of:

<vendor>,<chip/soc>-<block name>

>>
>> I'm more of an ACPI person than DTS.
>
>
> I think Rob is talking about something like this:
>
>         compatible="qcom,hidma-mgmt-1.0", "qcom,hidma-mgmt"
>
> This specifies that this is the v1.0 of the HIDMA management engine (or, the
> management engine for the 1.0 HIDMA device).  That way, if in the future
> there's a v1.1, you can do this:
>
>         compatible="qcom,hidma-mgmt-1.1", "qcom,hidma-mgmt"

Except I was suggesting not using 1.0 or 1.1. There is one main
exception and that is Xilinx blocks, but they are releasing versions
of blocks to customers. If "1.0" is not a well defined number, then
don't use that. I'd be surprised if any SOC vendor had such well
defined process around versioning of their IP blocks such that they
are well documented and guaranteed such that every change will change
the version.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Timur Tabi Nov. 2, 2015, 5:48 p.m. UTC | #5
On 11/02/2015 11:42 AM, Rob Herring wrote:
> On Mon, Nov 2, 2015 at 11:26 AM, Timur Tabi<timur@codeaurora.org>  wrote:
>> >On 11/02/2015 10:20 AM, Sinan Kaya wrote:
>>>> >>>
>>>> >>>
>>> >>Is there a good example I can look or a wiki about the device-tree
>>> >>naming conventions?
> There are many examples. Generally, it is the form of:
>
> <vendor>,<chip/soc>-<block name>
>

The problem is that we don't actually have an official name for this 
chip yet.
Rob Herring Nov. 2, 2015, 6:25 p.m. UTC | #6
On Mon, Nov 2, 2015 at 11:48 AM, Timur Tabi <timur@codeaurora.org> wrote:
> On 11/02/2015 11:42 AM, Rob Herring wrote:
>>
>> On Mon, Nov 2, 2015 at 11:26 AM, Timur Tabi<timur@codeaurora.org>  wrote:
>>>
>>> >On 11/02/2015 10:20 AM, Sinan Kaya wrote:
>>>>>
>>>>> >>>
>>>>> >>>
>>>>
>>>> >>Is there a good example I can look or a wiki about the device-tree
>>>> >>naming conventions?
>>
>> There are many examples. Generally, it is the form of:
>>
>> <vendor>,<chip/soc>-<block name>
>>
>
> The problem is that we don't actually have an official name for this chip
> yet.

Then document it with "<chip>" and fill that in later. Just don't make
up version numbers.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Timur Tabi Nov. 2, 2015, 6:30 p.m. UTC | #7
On 11/02/2015 12:25 PM, Rob Herring wrote:
> Then document it with "<chip>" and fill that in later. Just don't make
> up version numbers.

I don't think you understand.  We literally have no name for our chip. 
The closest is what I used on the pin control driver, "qdf2xxx", which 
really doesn't say anything.

	"qcom,qdf2xxx-hidma-mgmt"

doesn't sound right.

As for version numbers, sorry, I don't know what I was thinking.  I 
wrote plenty of device trees the way you suggest, I just forgot about them.
Sinan Kaya Nov. 2, 2015, 6:49 p.m. UTC | #8
On 11/2/2015 12:42 PM, Rob Herring wrote:
> Except I was suggesting not using 1.0 or 1.1. There is one main
> exception and that is Xilinx blocks, but they are releasing versions
> of blocks to customers. If "1.0" is not a well defined number, then
> don't use that. I'd be surprised if any SOC vendor had such well
> defined process around versioning of their IP blocks such that they
> are well documented and guaranteed such that every change will change
> the version.

Here is one.

I have two versions of the same IP. The first version in one chip has 
sw_version register that returns 1.0. The second version which has more 
capabilities has 1.1 in it.

Is it OK to use?

compatible="qcom,hidma-mgmt-1.0", "qcom,hidma-mgmt"

for now and

compatible="qcom,hidma-mgmt-1.1", "qcom,hidma-mgmt"

later for the second chip? 1.1 is backwards compatible with 1.0 BTW.

Since the same IP goes into multiple chips, why would you list the chip 
name here and submit patches multiple times for each single chip.

or to follow what Timur did, I can do this.

"qcom,qdf2xxx-hidma-mgmt-1.0"

qdf2xxx would become the chip family.
Arnd Bergmann Nov. 2, 2015, 10 p.m. UTC | #9
On Monday 02 November 2015 13:49:35 Sinan Kaya wrote:
> On 11/2/2015 12:42 PM, Rob Herring wrote:
> > Except I was suggesting not using 1.0 or 1.1. There is one main
> > exception and that is Xilinx blocks, but they are releasing versions
> > of blocks to customers. If "1.0" is not a well defined number, then
> > don't use that. I'd be surprised if any SOC vendor had such well
> > defined process around versioning of their IP blocks such that they
> > are well documented and guaranteed such that every change will change
> > the version.
> 
> Here is one.
> 
> I have two versions of the same IP. The first version in one chip has 
> sw_version register that returns 1.0. The second version which has more 
> capabilities has 1.1 in it.
> 
> Is it OK to use?
> 
> compatible="qcom,hidma-mgmt-1.0", "qcom,hidma-mgmt"
> 
> for now and
> 
> compatible="qcom,hidma-mgmt-1.1", "qcom,hidma-mgmt"
> 
> later for the second chip? 1.1 is backwards compatible with 1.0 BTW.

I think this is fine. As they are backwards compatible, I would even make the
latter one

compatible = "qcom,hidma-mgmt-1.1", "qcom,hidma-mgmt-1.0", "qcom,hidma-mgmt";

> Since the same IP goes into multiple chips, why would you list the chip 
> name here and submit patches multiple times for each single chip.
> 
> or to follow what Timur did, I can do this.
> 
> "qcom,qdf2xxx-hidma-mgmt-1.0"
> 
> qdf2xxx would become the chip family.

We really don't want wildcards in here, but want to use the most specific
name you have for it, so we can add quirks to the driver later if it
turns out that they are not fully compatible after all.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sinan Kaya Nov. 3, 2015, 5:18 a.m. UTC | #10
On 11/2/2015 10:57 AM, Rob Herring wrote:
> On Mon, Nov 2, 2015 at 12:07 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> The Qualcomm Technologies HIDMA device has been designed
>> to support virtualization technology. The driver has been
>> divided into two to follow the hardware design. The management
>> driver is executed in hypervisor context and is the main
>> management entity for all channels provided by the device.
>> The channel driver is executed in the hypervisor/guest OS
>> context.
>>
>> All channel devices get probed in the hypervisor
>> context during power up. They show up as DMA engine
>> channels. Then, before starting the virtualization; each
>> channel device is unbound from the hypervisor by VFIO
>> and assigned to the guest machine for control.
>>
>> This management driver will be used by the system
>> admin to monitor/reset the execution state of the DMA
>> channels. This will be the management interface.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>   .../devicetree/bindings/dma/qcom_hidma_mgmt.txt    |  56 ++
>>   drivers/dma/Kconfig                                |  11 +
>>   drivers/dma/Makefile                               |   1 +
>>   drivers/dma/qcom_hidma_mgmt.c                      | 747 +++++++++++++++++++++
>>   4 files changed, 815 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
>>   create mode 100644 drivers/dma/qcom_hidma_mgmt.c
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
>> new file mode 100644
>> index 0000000..514d37d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
>> @@ -0,0 +1,56 @@
>> +Qualcomm Technologies HIDMA Management interface
>> +
>> +The Qualcomm Technologies HIDMA device has been designed
>> +to support virtualization technology. The driver has been
>> +divided into two to follow the hardware design. The management
>> +driver is executed in hypervisor context and is the main
>> +management entity for all channels provided by the device.

Let me try one more time. Hope this helps.

Each HIDMA HW consists of multiple channels. These channels share
some set of common parameters. These parameters are initialized
by the management driver during power up. Same management driver is used 
for monitoring the execution of the channels. Management driver
can change the performance behaviors dynamically such as bandwidth
allocation and prioritization.

>
> This doesn't really explain what the block is.
>
>> +All channel devices get probed in the hypervisor
>> +context during power up. They show up as DMA engine
>> +DMA channels. Then, before starting the virtualization; each
>> +channel device is unbound from the hypervisor by VFIO
>> +and assign to the guest machine for control.
>> +
>> +This management driver will  be used by the system
>> +admin to monitor/reset the execution state of the DMA
>> +channels. This will be the management interface.
>> +
>> +
>> +Required properties:
>> +- compatible: must contain "qcom,hidma-mgmt"
>
> Please make this more specific. It doesn't match your example either.
> Unless "1.0" type versioning is tightly controlled and defined,
> version numbers are usually not a good practice.
>
>> +- reg: Address range for DMA device
>> +- dma-channels: Number of channels supported by this DMA controller.
>> +- max-write-burst-bytes: Maximum write burst in bytes. A memcpy requested is
>> +  fragmented to multiples of this amount.
>> +- max-read-burst-bytes: Maximum read burst in bytes. A memcpy request is
>> +  fragmented to multiples of this amount.
>> +- max-write-transactions: Maximum write transactions to perform in a burst
>> +- max-read-transactions: Maximum read transactions to perform in a burst
>
> This would be a function of burst-bytes and bus width. Are you sure
> you don't me number of outstanding transactions which is a common
> parameter for AXI bus peripherals.
>

These are all the available HW knobs for performance at this moment. 
max-write-transactions corresponds to the max number of outstanding 
transactions.

>> +- channel-reset-timeout: Channel reset timeout for this SOC.
>
> Please add units to property name.

ok, changed to channel-reset-timeout-cycles

>
>> +- channel-priority: Priority of the channel.
>> +  Each dma channel share the same HW bandwidth with other dma channels.
>> +  If two requests reach to the HW at the same time from a low priority and
>> +  high priority channel, high priority channel will claim the bus.
>> +  0=low priority, 1=high priority
>> +- channel-weight: Round robin weight of the channel
>> +  Since there are only two priority levels supported, scheduling among
>> +  the equal priority channels is done via weights.
>> +
>> +Example:
>> +
>> +       hidma-mgmt@f9984000 = {
>> +               compatible = "qcom,hidma-mgmt-1.0";
>> +               reg = <0xf9984000 0x15000>;
>> +               dma-channels = 6;
>> +               max-write-burst-bytes = 1024;
>> +               max-read-burst-bytes = 1024;
>> +               max-write-transactions = 31;
>> +               max-read-transactions = 31;
>> +               channel-reset-timeout = 0x500;
>> +               channel-priority = < 1 1 0 0 0 0>;
>> +               channel-weight = < 1 13 10 3 4 5>;
>> +       };
>> +
Andy Shevchenko Nov. 3, 2015, 10:22 a.m. UTC | #11
On Mon, Nov 2, 2015 at 8:07 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> The Qualcomm Technologies HIDMA device has been designed
> to support virtualization technology. The driver has been
> divided into two to follow the hardware design. The management
> driver is executed in hypervisor context and is the main
> management entity for all channels provided by the device.
> The channel driver is executed in the hypervisor/guest OS
> context.
>
> All channel devices get probed in the hypervisor
> context during power up. They show up as DMA engine
> channels. Then, before starting the virtualization; each
> channel device is unbound from the hypervisor by VFIO
> and assigned to the guest machine for control.
>
> This management driver will be used by the system
> admin to monitor/reset the execution state of the DMA
> channels. This will be the management interface.


> +static ssize_t qcom_hidma_mgmt_evtena(struct file *file,
> +       const char __user *user_buf, size_t count, loff_t *ppos)
> +{
> +       char temp_buf[16+1];

16 is a magic number. Make it defined constant.

> +       struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
> +       u32 event;
> +       ssize_t ret;
> +       unsigned long val;
> +
> +       temp_buf[16] = '\0';
> +       if (copy_from_user(temp_buf, user_buf, min_t(int, count, 16)))
> +               goto out;
> +
> +       ret = kstrtoul(temp_buf, 16, &val);

kstrtoul_from_user?

> +       if (ret) {
> +               dev_warn(&mgmtdev->pdev->dev, "unknown event\n");
> +               goto out;
> +       }
> +
> +       event = (u32)val & HW_EVENTS_CFG_MASK;
> +
> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
> +       writel(event, mgmtdev->dev_virtaddr + HW_EVENTS_CFG_OFFSET);
> +       pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
> +       pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
> +out:
> +       return count;
> +}
> +
> +static const struct file_operations qcom_hidma_mgmt_evtena_fops = {
> +       .write = qcom_hidma_mgmt_evtena,
> +};
> +
> +struct fileinfo {
> +       char *name;
> +       int mode;
> +       const struct file_operations *ops;
> +};
> +
> +static struct fileinfo files[] = {
> +       {"info", S_IRUGO, &qcom_hidma_mgmt_fops},
> +       {"err", S_IRUGO,  &qcom_hidma_mgmt_err_fops},
> +       {"mhiderrclr", S_IWUSR, &qcom_hidma_mgmt_mhiderr_clrfops},
> +       {"evterrclr", S_IWUSR, &qcom_hidma_mgmt_evterr_clrfops},
> +       {"ideerrclr", S_IWUSR, &qcom_hidma_mgmt_ideerr_clrfops},
> +       {"odeerrclr", S_IWUSR, &qcom_hidma_mgmt_odeerr_clrfops},
> +       {"msierrclr", S_IWUSR, &qcom_hidma_mgmt_msierr_clrfops},
> +       {"treerrclr", S_IWUSR, &qcom_hidma_mgmt_treerr_clrfops},
> +       {"evtena", S_IWUSR, &qcom_hidma_mgmt_evtena_fops},
> +};
> +
> +static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
> +{
> +       debugfs_remove_recursive(mgmtdev->debugfs);
> +}
> +
> +static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
> +{
> +       int rc = 0;
> +       u32 i;
> +       struct dentry   *fs_entry;
> +
> +       mgmtdev->debugfs = debugfs_create_dir(dev_name(&mgmtdev->pdev->dev),
> +                                               NULL);
> +       if (!mgmtdev->debugfs) {
> +               rc = -ENODEV;
> +               return rc;
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(files); i++) {
> +               fs_entry = debugfs_create_file(files[i].name,
> +                                       files[i].mode, mgmtdev->debugfs,
> +                                       mgmtdev, files[i].ops);
> +               if (!fs_entry) {
> +                       rc = -ENOMEM;
> +                       goto cleanup;
> +               }
> +       }
> +
> +       return 0;
> +cleanup:
> +       qcom_hidma_mgmt_debug_uninit(mgmtdev);
> +       return rc;
> +}
> +#else
> +static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
> +{
> +}
> +static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
> +{
> +       return 0;
> +}
> +#endif
> +
> +static int qcom_hidma_mgmt_setup(struct qcom_hidma_mgmt_dev *mgmtdev)
> +{
> +       u32 val;
> +       u32 i;
> +
> +       val = readl(mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
> +       val = val &
> +               ~(MAX_BUS_REQ_LEN_MASK << MAX_BUS_WR_REQ_BIT_POS);
> +       val = val |
> +               (mgmtdev->max_write_request << MAX_BUS_WR_REQ_BIT_POS);
> +       val = val & ~(MAX_BUS_REQ_LEN_MASK);
> +       val = val | (mgmtdev->max_read_request);
> +       writel(val, mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
> +
> +       val = readl(mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
> +       val = val &
> +               ~(MAX_WR_XACTIONS_MASK << MAX_WR_XACTIONS_BIT_POS);
> +       val = val |
> +               (mgmtdev->max_wr_xactions << MAX_WR_XACTIONS_BIT_POS);
> +       val = val & ~(MAX_RD_XACTIONS_MASK);
> +       val = val | (mgmtdev->max_rd_xactions);
> +       writel(val, mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
> +
> +       mgmtdev->sw_version = readl(mgmtdev->dev_virtaddr + SW_VERSION_OFFSET);
> +
> +       for (i = 0; i < mgmtdev->dma_channels; i++) {
> +               val = readl(mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
> +               val = val & ~(1 << PRIORITY_BIT_POS);
> +               val = val |
> +                       ((mgmtdev->priority[i] & 0x1) << PRIORITY_BIT_POS);
> +               val = val & ~(WEIGHT_MASK << WRR_BIT_POS);
> +               val = val
> +                       | ((mgmtdev->weight[i] & WEIGHT_MASK) << WRR_BIT_POS);
> +               writel(val, mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
> +       }
> +
> +       val = readl(mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
> +       val = val & ~CHRESET_TIMEOUUT_MASK;
> +       val = val | (mgmtdev->chreset_timeout & CHRESET_TIMEOUUT_MASK);
> +       writel(val, mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
> +
> +       val = readl(mgmtdev->dev_virtaddr + CFG_OFFSET);
> +       val = val | 1;
> +       writel(val, mgmtdev->dev_virtaddr + CFG_OFFSET);
> +
> +       return 0;
> +}
> +
> +static int qcom_hidma_mgmt_probe(struct platform_device *pdev)
> +{
> +       struct resource *dma_resource;
> +       int irq;
> +       int rc;
> +       u32 i;
> +       struct qcom_hidma_mgmt_dev *mgmtdev;

Better move this line to the top of definition block.

> +
> +       pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
> +       pm_runtime_use_autosuspend(&pdev->dev);
> +       pm_runtime_set_active(&pdev->dev);
> +       pm_runtime_enable(&pdev->dev);
> +       dma_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!dma_resource) {
> +               dev_err(&pdev->dev, "No memory resources found\n");
> +               rc = -ENODEV;
> +               goto out;
> +       }

Consolidate with devm_ioremap_resource()

> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0) {
> +               dev_err(&pdev->dev, "irq resources not found\n");
> +               rc = -ENODEV;
> +               goto out;
> +       }
> +
> +       mgmtdev = devm_kzalloc(&pdev->dev, sizeof(*mgmtdev), GFP_KERNEL);
> +       if (!mgmtdev) {
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       mgmtdev->pdev = pdev;
> +
> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
> +       mgmtdev->dev_addrsize = resource_size(dma_resource);
> +       mgmtdev->dev_virtaddr = devm_ioremap_resource(&pdev->dev,
> +                                                       dma_resource);
> +       if (IS_ERR(mgmtdev->dev_virtaddr)) {
> +               dev_err(&pdev->dev, "can't map i/o memory at %pa\n",
> +                       &dma_resource->start);
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "dma-channels",
> +               &mgmtdev->dma_channels)) {
> +               dev_err(&pdev->dev, "number of channels missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "channel-reset-timeout",
> +               &mgmtdev->chreset_timeout)) {
> +               dev_err(&pdev->dev, "channel reset timeout missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "max-write-burst-bytes",
> +               &mgmtdev->max_write_request)) {
> +               dev_err(&pdev->dev, "max-write-burst-bytes missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +       if ((mgmtdev->max_write_request != 128) &&
> +               (mgmtdev->max_write_request != 256) &&
> +               (mgmtdev->max_write_request != 512) &&
> +               (mgmtdev->max_write_request != 1024)) {

is_power_of_2()  && min/max ?

> +               dev_err(&pdev->dev, "invalid write request %d\n",
> +                       mgmtdev->max_write_request);
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "max-read-burst-bytes",
> +               &mgmtdev->max_read_request)) {
> +               dev_err(&pdev->dev, "max-read-burst-bytes missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if ((mgmtdev->max_read_request != 128) &&
> +               (mgmtdev->max_read_request != 256) &&
> +               (mgmtdev->max_read_request != 512) &&
> +               (mgmtdev->max_read_request != 1024)) {

Ditto.

> +               dev_err(&pdev->dev, "invalid read request %d\n",
> +                       mgmtdev->max_read_request);
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "max-write-transactions",
> +               &mgmtdev->max_wr_xactions)) {
> +               dev_err(&pdev->dev, "max-write-transactions missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32(&pdev->dev, "max-read-transactions",
> +               &mgmtdev->max_rd_xactions)) {
> +               dev_err(&pdev->dev, "max-read-transactions missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       mgmtdev->priority = devm_kcalloc(&pdev->dev,
> +               mgmtdev->dma_channels, sizeof(*mgmtdev->priority), GFP_KERNEL);
> +       if (!mgmtdev->priority) {
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       mgmtdev->weight = devm_kcalloc(&pdev->dev,
> +               mgmtdev->dma_channels, sizeof(*mgmtdev->weight), GFP_KERNEL);
> +       if (!mgmtdev->weight) {
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32_array(&pdev->dev, "channel-priority",
> +                               mgmtdev->priority, mgmtdev->dma_channels)) {
> +               dev_err(&pdev->dev, "channel-priority missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (device_property_read_u32_array(&pdev->dev, "channel-weight",
> +                               mgmtdev->weight, mgmtdev->dma_channels)) {
> +               dev_err(&pdev->dev, "channel-weight missing\n");
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       for (i = 0; i < mgmtdev->dma_channels; i++) {
> +               if (mgmtdev->weight[i] > 15) {

15 is magic.

> +                       dev_err(&pdev->dev,
> +                               "max value of weight can be 15.\n");
> +                       rc = -EINVAL;
> +                       goto out;
> +               }
> +
> +               /* weight needs to be at least one */
> +               if (mgmtdev->weight[i] == 0)
> +                       mgmtdev->weight[i] = 1;
> +       }
> +
> +       rc = qcom_hidma_mgmt_setup(mgmtdev);
> +       if (rc) {
> +               dev_err(&pdev->dev, "setup failed\n");
> +               goto out;
> +       }
> +
> +       rc = qcom_hidma_mgmt_debug_init(mgmtdev);
> +       if (rc) {
> +               dev_err(&pdev->dev, "debugfs init failed\n");
> +               goto out;
> +       }
> +
> +       dev_info(&pdev->dev,
> +               "HI-DMA engine management driver registration complete\n");

You may put some useful information here, otherwise looks like a debug message.

> +       platform_set_drvdata(pdev, mgmtdev);
> +       pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
> +       pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
> +       return 0;
> +out:
> +       pm_runtime_disable(&pdev->dev);
> +       pm_runtime_put_sync_suspend(&pdev->dev);
> +       return rc;
> +}
> +
> +static int qcom_hidma_mgmt_remove(struct platform_device *pdev)
> +{
> +       struct qcom_hidma_mgmt_dev *mgmtdev = platform_get_drvdata(pdev);
> +
> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
> +       qcom_hidma_mgmt_debug_uninit(mgmtdev);
> +       pm_runtime_put_sync_suspend(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
> +       dev_info(&pdev->dev, "HI-DMA engine management driver removed\n");

Useless message.

> +       return 0;
> +}
> +
> +#if IS_ENABLED(CONFIG_ACPI)
> +static const struct acpi_device_id qcom_hidma_mgmt_acpi_ids[] = {
> +       {"QCOM8060"},
> +       {},
> +};
> +#endif
> +
> +static const struct of_device_id qcom_hidma_mgmt_match[] = {
> +       { .compatible = "qcom,hidma-mgmt-1.0", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, qcom_hidma_mgmt_match);
> +
> +static struct platform_driver qcom_hidma_mgmt_driver = {
> +       .probe = qcom_hidma_mgmt_probe,
> +       .remove = qcom_hidma_mgmt_remove,
> +       .driver = {
> +               .name = "hidma-mgmt",
> +               .of_match_table = qcom_hidma_mgmt_match,
> +               .acpi_match_table = ACPI_PTR(qcom_hidma_mgmt_acpi_ids),
> +       },
> +};
> +module_platform_driver(qcom_hidma_mgmt_driver);
> +MODULE_LICENSE("GPL v2");
> --
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Sinan Kaya Nov. 4, 2015, 12:47 a.m. UTC | #12
On 11/3/2015 5:22 AM, Andy Shevchenko wrote:
> On Mon, Nov 2, 2015 at 8:07 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> The Qualcomm Technologies HIDMA device has been designed
>> to support virtualization technology. The driver has been
>> divided into two to follow the hardware design. The management
>> driver is executed in hypervisor context and is the main
>> management entity for all channels provided by the device.
>> The channel driver is executed in the hypervisor/guest OS
>> context.
>>
>> All channel devices get probed in the hypervisor
>> context during power up. They show up as DMA engine
>> channels. Then, before starting the virtualization; each
>> channel device is unbound from the hypervisor by VFIO
>> and assigned to the guest machine for control.
>>
>> This management driver will be used by the system
>> admin to monitor/reset the execution state of the DMA
>> channels. This will be the management interface.
>
>
>> +static ssize_t qcom_hidma_mgmt_evtena(struct file *file,
>> +       const char __user *user_buf, size_t count, loff_t *ppos)
>> +{
>> +       char temp_buf[16+1];
>
> 16 is a magic number. Make it defined constant.
>

removed it

>> +       struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
>> +       u32 event;
>> +       ssize_t ret;
>> +       unsigned long val;
>> +
>> +       temp_buf[16] = '\0';
>> +       if (copy_from_user(temp_buf, user_buf, min_t(int, count, 16)))
>> +               goto out;
>> +
>> +       ret = kstrtoul(temp_buf, 16, &val);
>
> kstrtoul_from_user?

nice, simpler.

>
>> +       if (ret) {
>> +               dev_warn(&mgmtdev->pdev->dev, "unknown event\n");
>> +               goto out;
>> +       }
>> +
>> +       event = (u32)val & HW_EVENTS_CFG_MASK;
>> +
>> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
>> +       writel(event, mgmtdev->dev_virtaddr + HW_EVENTS_CFG_OFFSET);
>> +       pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
>> +       pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
>> +out:
>> +       return count;
>> +}
>> +
>> +static const struct file_operations qcom_hidma_mgmt_evtena_fops = {
>> +       .write = qcom_hidma_mgmt_evtena,
>> +};
>> +
>> +struct fileinfo {
>> +       char *name;
>> +       int mode;
>> +       const struct file_operations *ops;
>> +};
>> +
>> +static struct fileinfo files[] = {
>> +       {"info", S_IRUGO, &qcom_hidma_mgmt_fops},
>> +       {"err", S_IRUGO,  &qcom_hidma_mgmt_err_fops},
>> +       {"mhiderrclr", S_IWUSR, &qcom_hidma_mgmt_mhiderr_clrfops},
>> +       {"evterrclr", S_IWUSR, &qcom_hidma_mgmt_evterr_clrfops},
>> +       {"ideerrclr", S_IWUSR, &qcom_hidma_mgmt_ideerr_clrfops},
>> +       {"odeerrclr", S_IWUSR, &qcom_hidma_mgmt_odeerr_clrfops},
>> +       {"msierrclr", S_IWUSR, &qcom_hidma_mgmt_msierr_clrfops},
>> +       {"treerrclr", S_IWUSR, &qcom_hidma_mgmt_treerr_clrfops},
>> +       {"evtena", S_IWUSR, &qcom_hidma_mgmt_evtena_fops},
>> +};
>> +
>> +static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
>> +{
>> +       debugfs_remove_recursive(mgmtdev->debugfs);
>> +}
>> +
>> +static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
>> +{
>> +       int rc = 0;
>> +       u32 i;
>> +       struct dentry   *fs_entry;
>> +
>> +       mgmtdev->debugfs = debugfs_create_dir(dev_name(&mgmtdev->pdev->dev),
>> +                                               NULL);
>> +       if (!mgmtdev->debugfs) {
>> +               rc = -ENODEV;
>> +               return rc;
>> +       }
>> +
>> +       for (i = 0; i < ARRAY_SIZE(files); i++) {
>> +               fs_entry = debugfs_create_file(files[i].name,
>> +                                       files[i].mode, mgmtdev->debugfs,
>> +                                       mgmtdev, files[i].ops);
>> +               if (!fs_entry) {
>> +                       rc = -ENOMEM;
>> +                       goto cleanup;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +cleanup:
>> +       qcom_hidma_mgmt_debug_uninit(mgmtdev);
>> +       return rc;
>> +}
>> +#else
>> +static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
>> +{
>> +}
>> +static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
>> +{
>> +       return 0;
>> +}
>> +#endif
>> +
>> +static int qcom_hidma_mgmt_setup(struct qcom_hidma_mgmt_dev *mgmtdev)
>> +{
>> +       u32 val;
>> +       u32 i;
>> +
>> +       val = readl(mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
>> +       val = val &
>> +               ~(MAX_BUS_REQ_LEN_MASK << MAX_BUS_WR_REQ_BIT_POS);
>> +       val = val |
>> +               (mgmtdev->max_write_request << MAX_BUS_WR_REQ_BIT_POS);
>> +       val = val & ~(MAX_BUS_REQ_LEN_MASK);
>> +       val = val | (mgmtdev->max_read_request);
>> +       writel(val, mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
>> +
>> +       val = readl(mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
>> +       val = val &
>> +               ~(MAX_WR_XACTIONS_MASK << MAX_WR_XACTIONS_BIT_POS);
>> +       val = val |
>> +               (mgmtdev->max_wr_xactions << MAX_WR_XACTIONS_BIT_POS);
>> +       val = val & ~(MAX_RD_XACTIONS_MASK);
>> +       val = val | (mgmtdev->max_rd_xactions);
>> +       writel(val, mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
>> +
>> +       mgmtdev->sw_version = readl(mgmtdev->dev_virtaddr + SW_VERSION_OFFSET);
>> +
>> +       for (i = 0; i < mgmtdev->dma_channels; i++) {
>> +               val = readl(mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
>> +               val = val & ~(1 << PRIORITY_BIT_POS);
>> +               val = val |
>> +                       ((mgmtdev->priority[i] & 0x1) << PRIORITY_BIT_POS);
>> +               val = val & ~(WEIGHT_MASK << WRR_BIT_POS);
>> +               val = val
>> +                       | ((mgmtdev->weight[i] & WEIGHT_MASK) << WRR_BIT_POS);
>> +               writel(val, mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
>> +       }
>> +
>> +       val = readl(mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
>> +       val = val & ~CHRESET_TIMEOUUT_MASK;
>> +       val = val | (mgmtdev->chreset_timeout & CHRESET_TIMEOUUT_MASK);
>> +       writel(val, mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
>> +
>> +       val = readl(mgmtdev->dev_virtaddr + CFG_OFFSET);
>> +       val = val | 1;
>> +       writel(val, mgmtdev->dev_virtaddr + CFG_OFFSET);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_hidma_mgmt_probe(struct platform_device *pdev)
>> +{
>> +       struct resource *dma_resource;
>> +       int irq;
>> +       int rc;
>> +       u32 i;
>> +       struct qcom_hidma_mgmt_dev *mgmtdev;
>
> Better move this line to the top of definition block.
>
done

>> +
>> +       pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
>> +       pm_runtime_use_autosuspend(&pdev->dev);
>> +       pm_runtime_set_active(&pdev->dev);
>> +       pm_runtime_enable(&pdev->dev);
>> +       dma_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       if (!dma_resource) {
>> +               dev_err(&pdev->dev, "No memory resources found\n");
>> +               rc = -ENODEV;
>> +               goto out;
>> +       }
>
> Consolidate with devm_ioremap_resource()
>

OK

>> +
>> +       irq = platform_get_irq(pdev, 0);
>> +       if (irq < 0) {
>> +               dev_err(&pdev->dev, "irq resources not found\n");
>> +               rc = -ENODEV;
>> +               goto out;
>> +       }
>> +
>> +       mgmtdev = devm_kzalloc(&pdev->dev, sizeof(*mgmtdev), GFP_KERNEL);
>> +       if (!mgmtdev) {
>> +               rc = -ENOMEM;
>> +               goto out;
>> +       }
>> +
>> +       mgmtdev->pdev = pdev;
>> +
>> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
>> +       mgmtdev->dev_addrsize = resource_size(dma_resource);
>> +       mgmtdev->dev_virtaddr = devm_ioremap_resource(&pdev->dev,
>> +                                                       dma_resource);
>> +       if (IS_ERR(mgmtdev->dev_virtaddr)) {
>> +               dev_err(&pdev->dev, "can't map i/o memory at %pa\n",
>> +                       &dma_resource->start);
>> +               rc = -ENOMEM;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "dma-channels",
>> +               &mgmtdev->dma_channels)) {
>> +               dev_err(&pdev->dev, "number of channels missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "channel-reset-timeout",
>> +               &mgmtdev->chreset_timeout)) {
>> +               dev_err(&pdev->dev, "channel reset timeout missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "max-write-burst-bytes",
>> +               &mgmtdev->max_write_request)) {
>> +               dev_err(&pdev->dev, "max-write-burst-bytes missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +       if ((mgmtdev->max_write_request != 128) &&
>> +               (mgmtdev->max_write_request != 256) &&
>> +               (mgmtdev->max_write_request != 512) &&
>> +               (mgmtdev->max_write_request != 1024)) {
>
> is_power_of_2()  && min/max ?
>
ok

>> +               dev_err(&pdev->dev, "invalid write request %d\n",
>> +                       mgmtdev->max_write_request);
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "max-read-burst-bytes",
>> +               &mgmtdev->max_read_request)) {
>> +               dev_err(&pdev->dev, "max-read-burst-bytes missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if ((mgmtdev->max_read_request != 128) &&
>> +               (mgmtdev->max_read_request != 256) &&
>> +               (mgmtdev->max_read_request != 512) &&
>> +               (mgmtdev->max_read_request != 1024)) {
>
> Ditto.
>
done

>> +               dev_err(&pdev->dev, "invalid read request %d\n",
>> +                       mgmtdev->max_read_request);
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "max-write-transactions",
>> +               &mgmtdev->max_wr_xactions)) {
>> +               dev_err(&pdev->dev, "max-write-transactions missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32(&pdev->dev, "max-read-transactions",
>> +               &mgmtdev->max_rd_xactions)) {
>> +               dev_err(&pdev->dev, "max-read-transactions missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       mgmtdev->priority = devm_kcalloc(&pdev->dev,
>> +               mgmtdev->dma_channels, sizeof(*mgmtdev->priority), GFP_KERNEL);
>> +       if (!mgmtdev->priority) {
>> +               rc = -ENOMEM;
>> +               goto out;
>> +       }
>> +
>> +       mgmtdev->weight = devm_kcalloc(&pdev->dev,
>> +               mgmtdev->dma_channels, sizeof(*mgmtdev->weight), GFP_KERNEL);
>> +       if (!mgmtdev->weight) {
>> +               rc = -ENOMEM;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32_array(&pdev->dev, "channel-priority",
>> +                               mgmtdev->priority, mgmtdev->dma_channels)) {
>> +               dev_err(&pdev->dev, "channel-priority missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       if (device_property_read_u32_array(&pdev->dev, "channel-weight",
>> +                               mgmtdev->weight, mgmtdev->dma_channels)) {
>> +               dev_err(&pdev->dev, "channel-weight missing\n");
>> +               rc = -EINVAL;
>> +               goto out;
>> +       }
>> +
>> +       for (i = 0; i < mgmtdev->dma_channels; i++) {
>> +               if (mgmtdev->weight[i] > 15) {
>
> 15 is magic.
>

I created a new macro called MAX_CHANNEL_WEIGHT.

>> +                       dev_err(&pdev->dev,
>> +                               "max value of weight can be 15.\n");
>> +                       rc = -EINVAL;
>> +                       goto out;
>> +               }
>> +
>> +               /* weight needs to be at least one */
>> +               if (mgmtdev->weight[i] == 0)
>> +                       mgmtdev->weight[i] = 1;
>> +       }
>> +
>> +       rc = qcom_hidma_mgmt_setup(mgmtdev);
>> +       if (rc) {
>> +               dev_err(&pdev->dev, "setup failed\n");
>> +               goto out;
>> +       }
>> +
>> +       rc = qcom_hidma_mgmt_debug_init(mgmtdev);
>> +       if (rc) {
>> +               dev_err(&pdev->dev, "debugfs init failed\n");
>> +               goto out;
>> +       }
>> +
>> +       dev_info(&pdev->dev,
>> +               "HI-DMA engine management driver registration complete\n");
>
> You may put some useful information here, otherwise looks like a debug message.
>
ok, I did now. I copied the syntax from another driver to here.

>> +       platform_set_drvdata(pdev, mgmtdev);
>> +       pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
>> +       pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
>> +       return 0;
>> +out:
>> +       pm_runtime_disable(&pdev->dev);
>> +       pm_runtime_put_sync_suspend(&pdev->dev);
>> +       return rc;
>> +}
>> +
>> +static int qcom_hidma_mgmt_remove(struct platform_device *pdev)
>> +{
>> +       struct qcom_hidma_mgmt_dev *mgmtdev = platform_get_drvdata(pdev);
>> +
>> +       pm_runtime_get_sync(&mgmtdev->pdev->dev);
>> +       qcom_hidma_mgmt_debug_uninit(mgmtdev);
>> +       pm_runtime_put_sync_suspend(&pdev->dev);
>> +       pm_runtime_disable(&pdev->dev);
>> +
>> +       dev_info(&pdev->dev, "HI-DMA engine management driver removed\n");
>
> Useless message.

removed .

>
>> +       return 0;
>> +}
>> +
>> +#if IS_ENABLED(CONFIG_ACPI)
>> +static const struct acpi_device_id qcom_hidma_mgmt_acpi_ids[] = {
>> +       {"QCOM8060"},
>> +       {},
>> +};
>> +#endif
>> +
>> +static const struct of_device_id qcom_hidma_mgmt_match[] = {
>> +       { .compatible = "qcom,hidma-mgmt-1.0", },
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_hidma_mgmt_match);
>> +
>> +static struct platform_driver qcom_hidma_mgmt_driver = {
>> +       .probe = qcom_hidma_mgmt_probe,
>> +       .remove = qcom_hidma_mgmt_remove,
>> +       .driver = {
>> +               .name = "hidma-mgmt",
>> +               .of_match_table = qcom_hidma_mgmt_match,
>> +               .acpi_match_table = ACPI_PTR(qcom_hidma_mgmt_acpi_ids),
>> +       },
>> +};
>> +module_platform_driver(qcom_hidma_mgmt_driver);
>> +MODULE_LICENSE("GPL v2");
>> --
>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>
Rob Herring Nov. 5, 2015, 2:31 p.m. UTC | #13
On Mon, Nov 2, 2015 at 12:30 PM, Timur Tabi <timur@codeaurora.org> wrote:
> On 11/02/2015 12:25 PM, Rob Herring wrote:
>>
>> Then document it with "<chip>" and fill that in later. Just don't make
>> up version numbers.
>
>
> I don't think you understand.  We literally have no name for our chip. The
> closest is what I used on the pin control driver, "qdf2xxx", which really
> doesn't say anything.
>
>         "qcom,qdf2xxx-hidma-mgmt"

I'm saying document it as "qcom,<chip>-hidma-mgmt" and when you have
the part number update the binding. Meanwhile push on the powers that
be to decide on a part number.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Timur Tabi Nov. 5, 2015, 2:43 p.m. UTC | #14
Rob Herring wrote:
> I'm saying document it as "qcom,<chip>-hidma-mgmt" and when you have
> the part number update the binding. Meanwhile push on the powers that
> be to decide on a part number.

Got it.  But we should we do about this:

static const struct of_device_id qcom_hidma_mgmt_match[] = {
	{ .compatible = "qcom,hidma-mgmt-1.0", },
	{},
};
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
new file mode 100644
index 0000000..514d37d
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
@@ -0,0 +1,56 @@ 
+Qualcomm Technologies HIDMA Management interface
+
+The Qualcomm Technologies HIDMA device has been designed
+to support virtualization technology. The driver has been
+divided into two to follow the hardware design. The management
+driver is executed in hypervisor context and is the main
+management entity for all channels provided by the device.
+The channel driver is executed in the hypervisor/guest OS
+context.
+
+All channel devices get probed in the hypervisor
+context during power up. They show up as DMA engine
+DMA channels. Then, before starting the virtualization; each
+channel device is unbound from the hypervisor by VFIO
+and assign to the guest machine for control.
+
+This management driver will  be used by the system
+admin to monitor/reset the execution state of the DMA
+channels. This will be the management interface.
+
+
+Required properties:
+- compatible: must contain "qcom,hidma-mgmt"
+- reg: Address range for DMA device
+- dma-channels: Number of channels supported by this DMA controller.
+- max-write-burst-bytes: Maximum write burst in bytes. A memcpy requested is
+  fragmented to multiples of this amount.
+- max-read-burst-bytes: Maximum read burst in bytes. A memcpy request is
+  fragmented to multiples of this amount.
+- max-write-transactions: Maximum write transactions to perform in a burst
+- max-read-transactions: Maximum read transactions to perform in a burst
+- channel-reset-timeout: Channel reset timeout for this SOC.
+- channel-priority: Priority of the channel.
+  Each dma channel share the same HW bandwidth with other dma channels.
+  If two requests reach to the HW at the same time from a low priority and
+  high priority channel, high priority channel will claim the bus.
+  0=low priority, 1=high priority
+- channel-weight: Round robin weight of the channel
+  Since there are only two priority levels supported, scheduling among
+  the equal priority channels is done via weights.
+
+Example:
+
+	hidma-mgmt@f9984000 = {
+		compatible = "qcom,hidma-mgmt-1.0";
+		reg = <0xf9984000 0x15000>;
+		dma-channels = 6;
+		max-write-burst-bytes = 1024;
+		max-read-burst-bytes = 1024;
+		max-write-transactions = 31;
+		max-read-transactions = 31;
+		channel-reset-timeout = 0x500;
+		channel-priority = < 1 1 0 0 0 0>;
+		channel-weight = < 1 13 10 3 4 5>;
+	};
+
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index b458475..4c6f0b5 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -501,6 +501,17 @@  config XGENE_DMA
 	help
 	  Enable support for the APM X-Gene SoC DMA engine.
 
+config QCOM_HIDMA_MGMT
+	tristate "Qualcomm Technologies HIDMA Management support"
+	select DMA_ENGINE
+	help
+	  Enable support for the Qualcomm Technologies HIDMA Management.
+	  Each DMA device requires one management interface driver
+	  for basic initialization before QCOM_HIDMA channel driver can
+	  start managing the channels. In a virtualized environment,
+	  the guest OS would run QCOM_HIDMA channel driver and the
+	  hypervisor would run the QCOM_HIDMA_MGMT management driver.
+
 config XILINX_VDMA
 	tristate "Xilinx AXI VDMA Engine"
 	depends on (ARCH_ZYNQ || MICROBLAZE)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 7711a71..3d25ffd 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -53,6 +53,7 @@  obj-$(CONFIG_PL330_DMA) += pl330.o
 obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/
 obj-$(CONFIG_PXA_DMA) += pxa_dma.o
 obj-$(CONFIG_QCOM_BAM_DMA) += qcom_bam_dma.o
+obj-$(CONFIG_QCOM_HIDMA_MGMT) += qcom_hidma_mgmt.o
 obj-$(CONFIG_RENESAS_DMA) += sh/
 obj-$(CONFIG_SIRF_DMA) += sirf-dma.o
 obj-$(CONFIG_STE_DMA40) += ste_dma40.o ste_dma40_ll.o
diff --git a/drivers/dma/qcom_hidma_mgmt.c b/drivers/dma/qcom_hidma_mgmt.c
new file mode 100644
index 0000000..7652702
--- /dev/null
+++ b/drivers/dma/qcom_hidma_mgmt.c
@@ -0,0 +1,747 @@ 
+/*
+ * Qualcomm Technologies HIDMA DMA engine Management interface
+ *
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/dmaengine.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/debugfs.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+
+#define MHICFG_OFFSET			0x10
+#define QOS_N_OFFSET			0x300
+#define CFG_OFFSET			0x400
+#define HW_PARAM_OFFSET		0x408
+#define MAX_BUS_REQ_LEN_OFFSET		0x41C
+#define MAX_XACTIONS_OFFSET		0x420
+#define SW_VERSION_OFFSET		0x424
+#define CHRESET_TIMEOUT_OFFSET		0x418
+#define MHID_BUS_ERR0_OFFSET		0x1020
+#define MHID_BUS_ERR1_OFFSET		0x1024
+#define MHID_BUS_ERR_CLR_OFFSET	0x102C
+#define EVT_BUS_ERR0_OFFSET		0x1030
+#define EVT_BUS_ERR1_OFFSET		0x1034
+#define EVT_BUS_ERR_CLR_OFFSET		0x103C
+#define IDE_BUS_ERR0_OFFSET		0x1040
+#define IDE_BUS_ERR1_OFFSET		0x1044
+#define IDE_BUS_ERR2_OFFSET		0x1048
+#define IDE_BUS_ERR_CLR_OFFSET		0x104C
+#define ODE_BUS_ERR0_OFFSET		0x1050
+#define ODE_BUS_ERR1_OFFSET		0x1054
+#define ODE_BUS_ERR2_OFFSET		0x1058
+#define ODE_BUS_ERR_CLR_OFFSET		0x105C
+#define MSI_BUS_ERR0_OFFSET		0x1060
+#define MSI_BUS_ERR_CLR_OFFSET		0x106C
+#define TRE_ERR0_OFFSET		0x1070
+#define TRE_ERR_CLR_OFFSET		0x107C
+#define HW_EVENTS_CFG_OFFSET		0x1080
+
+#define HW_EVENTS_CFG_MASK		0xFF
+#define TRE_ERR_TRCHID_MASK		0xF
+#define TRE_ERR_EVRIDX_MASK		0xFF
+#define TRE_ERR_TYPE_MASK		0xFF
+#define MSI_ERR_RESP_MASK		0xFF
+#define MSI_ERR_TRCHID_MASK		0xFF
+#define ODE_ERR_REQLEN_MASK		0xFFFF
+#define ODE_ERR_RESP_MASK		0xFF
+#define ODE_ERR_TRCHID_MASK		0xFF
+#define IDE_ERR_REQLEN_MASK		0xFFFF
+#define IDE_ERR_RESP_MASK		0xFF
+#define IDE_ERR_TRCHID_MASK		0xFF
+#define EVT_ERR_RESP_MASK		0xFF
+#define EVT_ERR_TRCHID_MASK		0xFF
+#define MHID_ERR_RESP_MASK		0xFF
+#define MHID_ERR_TRCHID_MASK		0xFF
+#define MAX_WR_XACTIONS_MASK		0x1F
+#define MAX_RD_XACTIONS_MASK		0x1F
+#define MAX_JOBSIZE_MASK		0xFF
+#define MAX_COIDX_MASK			0xFF
+#define TREQ_CAPACITY_MASK		0xFF
+#define WEIGHT_MASK			0x7F
+#define TREQ_LIMIT_MASK		0x1FF
+#define NR_CHANNEL_MASK		0xFFFF
+#define MAX_BUS_REQ_LEN_MASK		0xFFFF
+#define CHRESET_TIMEOUUT_MASK		0xFFFFF
+
+#define TRE_ERR_TRCHID_BIT_POS		28
+#define TRE_ERR_IEOB_BIT_POS		16
+#define TRE_ERR_EVRIDX_BIT_POS		8
+#define MSI_ERR_RESP_BIT_POS		8
+#define ODE_ERR_REQLEN_BIT_POS		16
+#define ODE_ERR_RESP_BIT_POS		8
+#define IDE_ERR_REQLEN_BIT_POS		16
+#define IDE_ERR_RESP_BIT_POS		8
+#define EVT_ERR_RESP_BIT_POS		8
+#define MHID_ERR_RESP_BIT_POS		8
+#define MAX_WR_XACTIONS_BIT_POS	16
+#define TREQ_CAPACITY_BIT_POS		8
+#define MAX_JOB_SIZE_BIT_POS		16
+#define NR_EV_CHANNEL_BIT_POS		16
+#define MAX_BUS_WR_REQ_BIT_POS		16
+#define WRR_BIT_POS			8
+#define PRIORITY_BIT_POS		15
+#define TREQ_LIMIT_BIT_POS		16
+#define TREQ_LIMIT_EN_BIT_POS		23
+#define STOP_BIT_POS			24
+
+#define AUTOSUSPEND_TIMEOUT		2000
+
+struct qcom_hidma_mgmt_dev {
+	u32 max_wr_xactions;
+	u32 max_rd_xactions;
+	u32 max_write_request;
+	u32 max_read_request;
+	u32 dma_channels;
+	u32 chreset_timeout;
+	u32 sw_version;
+	u32 *priority;
+	u32 *weight;
+
+	/* Hardware device constants */
+	void __iomem *dev_virtaddr;
+	resource_size_t dev_addrsize;
+
+	struct dentry	*debugfs;
+	struct platform_device *pdev;
+};
+
+static unsigned int debug_pm;
+module_param(debug_pm, uint, 0644);
+MODULE_PARM_DESC(debug_pm,
+		 "debug runtime power management transitions (default: 0)");
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+static inline void HIDMA_READ_SHOW(struct seq_file *s,
+		void __iomem *dev_virtaddr, char *name, int offset)
+{
+	u32 val;
+
+	val = readl(dev_virtaddr + offset);
+	seq_printf(s, "%s=0x%x\n", name, val);
+}
+
+/**
+ * qcom_hidma_mgmt_info: display HIDMA device info
+ *
+ * Display the info for the current HIDMA device.
+ */
+static int qcom_hidma_mgmt_info(struct seq_file *s, void *unused)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = s->private;
+	u32 val;
+	u32 i;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	seq_printf(s, "sw_version=0x%x\n", mgmtdev->sw_version);
+
+	val = readl(mgmtdev->dev_virtaddr + CFG_OFFSET);
+	seq_printf(s, "ENABLE=%d\n", val & 0x1);
+
+	val = readl(mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
+	seq_printf(s, "reset_timeout=%d\n", val & CHRESET_TIMEOUUT_MASK);
+
+	val = readl(mgmtdev->dev_virtaddr + MHICFG_OFFSET);
+	seq_printf(s, "nr_event_channel=%d\n",
+		(val >> NR_EV_CHANNEL_BIT_POS) & NR_CHANNEL_MASK);
+	seq_printf(s, "nr_tr_channel=%d\n", (val & NR_CHANNEL_MASK));
+	seq_printf(s, "dma_channels=%d\n", mgmtdev->dma_channels);
+	seq_printf(s, "dev_addrsize=%pap\n", &mgmtdev->dev_addrsize);
+
+	val = readl(mgmtdev->dev_virtaddr + HW_PARAM_OFFSET);
+	seq_printf(s, "MAX_JOB_SIZE=%d\n",
+		(val >> MAX_JOB_SIZE_BIT_POS) & MAX_JOBSIZE_MASK);
+	seq_printf(s, "TREQ_CAPACITY=%d\n",
+		(val >> TREQ_CAPACITY_BIT_POS) & TREQ_CAPACITY_MASK);
+	seq_printf(s, "MAX_COIDX_DEPTH=%d\n", val & MAX_COIDX_MASK);
+
+	val = readl(mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
+	seq_printf(s, "MAX_BUS_WR_REQ_LEN=%d\n",
+		(val >> MAX_BUS_WR_REQ_BIT_POS) & MAX_BUS_REQ_LEN_MASK);
+	seq_printf(s, "MAX_BUS_RD_REQ_LEN=%d\n", val & MAX_BUS_REQ_LEN_MASK);
+
+	val = readl(mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
+	seq_printf(s, "MAX_WR_XACTIONS=%d\n",
+		(val >> MAX_WR_XACTIONS_BIT_POS) & MAX_WR_XACTIONS_MASK);
+	seq_printf(s, "MAX_RD_XACTIONS=%d\n", val & MAX_RD_XACTIONS_MASK);
+
+	for (i = 0; i < mgmtdev->dma_channels; i++) {
+		void __iomem *offset;
+
+		offset = mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i);
+		val = readl(offset);
+
+		seq_printf(s, "CH#%d STOP=%d\n",
+			i, (val & (1 << STOP_BIT_POS)) ? 1 : 0);
+		seq_printf(s, "CH#%d TREQ LIMIT EN=%d\n", i,
+			(val & (1 << TREQ_LIMIT_EN_BIT_POS)) ? 1 : 0);
+		seq_printf(s, "CH#%d TREQ LIMIT=%d\n",
+			i, (val >> TREQ_LIMIT_BIT_POS) & TREQ_LIMIT_MASK);
+		seq_printf(s, "CH#%d priority=%d\n", i,
+			(val & (1 << PRIORITY_BIT_POS)) ? 1 : 0);
+		seq_printf(s, "CH#%d WRR=%d\n", i,
+			(val >> WRR_BIT_POS) & WEIGHT_MASK);
+		seq_printf(s, "CH#%d USE_DLA=%d\n", i, (val & 1) ? 1 : 0);
+	}
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+
+	return 0;
+}
+
+static int qcom_hidma_mgmt_info_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, qcom_hidma_mgmt_info, inode->i_private);
+}
+
+static const struct file_operations qcom_hidma_mgmt_fops = {
+	.open = qcom_hidma_mgmt_info_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+/**
+ * qcom_hidma_mgmt_err: display HIDMA error info
+ *
+ * Display the error info for the current HIDMA device.
+ */
+static int qcom_hidma_mgmt_err(struct seq_file *s, void *unused)
+{
+	u32 val;
+	struct qcom_hidma_mgmt_dev *mgmtdev = s->private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	val = readl(mgmtdev->dev_virtaddr + MHID_BUS_ERR0_OFFSET);
+	seq_printf(s, "MHID TR_CHID=%d\n", val & MHID_ERR_TRCHID_MASK);
+	seq_printf(s, "MHID RESP_ERROR=%d\n",
+		(val >> MHID_ERR_RESP_BIT_POS) & MHID_ERR_RESP_MASK);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "MHID READ_PTR",
+		MHID_BUS_ERR1_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + EVT_BUS_ERR0_OFFSET);
+	seq_printf(s, "EVT TR_CHID=%d\n", val & EVT_ERR_TRCHID_MASK);
+	seq_printf(s, "EVT RESP_ERROR=%d\n",
+		(val >> EVT_ERR_RESP_BIT_POS) & EVT_ERR_RESP_MASK);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "EVT WRITE_PTR",
+		EVT_BUS_ERR1_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + IDE_BUS_ERR0_OFFSET);
+	seq_printf(s, "IDE TR_CHID=%d\n", val & IDE_ERR_TRCHID_MASK);
+	seq_printf(s, "IDE RESP_ERROR=%d\n",
+		(val >> IDE_ERR_RESP_BIT_POS) & IDE_ERR_RESP_MASK);
+	seq_printf(s, "IDE REQ_LENGTH=%d\n",
+		(val >> IDE_ERR_REQLEN_BIT_POS) & IDE_ERR_REQLEN_MASK);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "IDE ADDR_LSB",
+		IDE_BUS_ERR1_OFFSET);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "IDE ADDR_MSB",
+		IDE_BUS_ERR2_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + ODE_BUS_ERR0_OFFSET);
+	seq_printf(s, "ODE TR_CHID=%d\n", val & ODE_ERR_TRCHID_MASK);
+	seq_printf(s, "ODE RESP_ERROR=%d\n",
+		(val >> ODE_ERR_RESP_BIT_POS) & ODE_ERR_RESP_MASK);
+	seq_printf(s, "ODE REQ_LENGTH=%d\n",
+		(val >> ODE_ERR_REQLEN_BIT_POS) & ODE_ERR_REQLEN_MASK);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "ODE ADDR_LSB",
+		ODE_BUS_ERR1_OFFSET);
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "ODE ADDR_MSB",
+		ODE_BUS_ERR2_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + MSI_BUS_ERR0_OFFSET);
+	seq_printf(s, "MSI TR_CHID=%d\n", val & MSI_ERR_TRCHID_MASK);
+	seq_printf(s, "MSI RESP_ERROR=%d\n",
+		(val >> MSI_ERR_RESP_BIT_POS) & MSI_ERR_RESP_MASK);
+
+	val = readl(mgmtdev->dev_virtaddr + TRE_ERR0_OFFSET);
+	seq_printf(s, "TRE TRE_TYPE=%d\n", val & TRE_ERR_TYPE_MASK);
+	seq_printf(s, "TRE TRE_EVRIDX=%d\n",
+		(val >> TRE_ERR_EVRIDX_BIT_POS) & TRE_ERR_EVRIDX_MASK);
+	seq_printf(s, "TRE TRE_IEOB=%d\n",
+		(val >> TRE_ERR_IEOB_BIT_POS) & 1);
+	seq_printf(s, "TRE TRCHID=%d\n",
+		(val >> TRE_ERR_TRCHID_BIT_POS) & TRE_ERR_TRCHID_MASK);
+
+	HIDMA_READ_SHOW(s, mgmtdev->dev_virtaddr, "HW_EVENTS_CFG_OFFSET",
+			HW_EVENTS_CFG_OFFSET);
+
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return 0;
+}
+
+static int qcom_hidma_mgmt_err_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, qcom_hidma_mgmt_err, inode->i_private);
+}
+
+static const struct file_operations qcom_hidma_mgmt_err_fops = {
+	.open = qcom_hidma_mgmt_err_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static ssize_t qcom_hidma_mgmt_mhiderr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + MHID_BUS_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_mhiderr_clrfops = {
+	.write = qcom_hidma_mgmt_mhiderr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_evterr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + EVT_BUS_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_evterr_clrfops = {
+	.write = qcom_hidma_mgmt_evterr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_ideerr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + IDE_BUS_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_ideerr_clrfops = {
+	.write = qcom_hidma_mgmt_ideerr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_odeerr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + ODE_BUS_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_odeerr_clrfops = {
+	.write = qcom_hidma_mgmt_odeerr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_msierr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + MSI_BUS_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_msierr_clrfops = {
+	.write = qcom_hidma_mgmt_msierr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_treerr_clr(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(1, mgmtdev->dev_virtaddr + TRE_ERR_CLR_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_treerr_clrfops = {
+	.write = qcom_hidma_mgmt_treerr_clr,
+};
+
+static ssize_t qcom_hidma_mgmt_evtena(struct file *file,
+	const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	char temp_buf[16+1];
+	struct qcom_hidma_mgmt_dev *mgmtdev = file->f_inode->i_private;
+	u32 event;
+	ssize_t ret;
+	unsigned long val;
+
+	temp_buf[16] = '\0';
+	if (copy_from_user(temp_buf, user_buf, min_t(int, count, 16)))
+		goto out;
+
+	ret = kstrtoul(temp_buf, 16, &val);
+	if (ret) {
+		dev_warn(&mgmtdev->pdev->dev, "unknown event\n");
+		goto out;
+	}
+
+	event = (u32)val & HW_EVENTS_CFG_MASK;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	writel(event, mgmtdev->dev_virtaddr + HW_EVENTS_CFG_OFFSET);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+out:
+	return count;
+}
+
+static const struct file_operations qcom_hidma_mgmt_evtena_fops = {
+	.write = qcom_hidma_mgmt_evtena,
+};
+
+struct fileinfo {
+	char *name;
+	int mode;
+	const struct file_operations *ops;
+};
+
+static struct fileinfo files[] = {
+	{"info", S_IRUGO, &qcom_hidma_mgmt_fops},
+	{"err", S_IRUGO,  &qcom_hidma_mgmt_err_fops},
+	{"mhiderrclr", S_IWUSR, &qcom_hidma_mgmt_mhiderr_clrfops},
+	{"evterrclr", S_IWUSR, &qcom_hidma_mgmt_evterr_clrfops},
+	{"ideerrclr", S_IWUSR, &qcom_hidma_mgmt_ideerr_clrfops},
+	{"odeerrclr", S_IWUSR, &qcom_hidma_mgmt_odeerr_clrfops},
+	{"msierrclr", S_IWUSR, &qcom_hidma_mgmt_msierr_clrfops},
+	{"treerrclr", S_IWUSR, &qcom_hidma_mgmt_treerr_clrfops},
+	{"evtena", S_IWUSR, &qcom_hidma_mgmt_evtena_fops},
+};
+
+static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
+{
+	debugfs_remove_recursive(mgmtdev->debugfs);
+}
+
+static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
+{
+	int rc = 0;
+	u32 i;
+	struct dentry	*fs_entry;
+
+	mgmtdev->debugfs = debugfs_create_dir(dev_name(&mgmtdev->pdev->dev),
+						NULL);
+	if (!mgmtdev->debugfs) {
+		rc = -ENODEV;
+		return rc;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(files); i++) {
+		fs_entry = debugfs_create_file(files[i].name,
+					files[i].mode, mgmtdev->debugfs,
+					mgmtdev, files[i].ops);
+		if (!fs_entry) {
+			rc = -ENOMEM;
+			goto cleanup;
+		}
+	}
+
+	return 0;
+cleanup:
+	qcom_hidma_mgmt_debug_uninit(mgmtdev);
+	return rc;
+}
+#else
+static void qcom_hidma_mgmt_debug_uninit(struct qcom_hidma_mgmt_dev *mgmtdev)
+{
+}
+static int qcom_hidma_mgmt_debug_init(struct qcom_hidma_mgmt_dev *mgmtdev)
+{
+	return 0;
+}
+#endif
+
+static int qcom_hidma_mgmt_setup(struct qcom_hidma_mgmt_dev *mgmtdev)
+{
+	u32 val;
+	u32 i;
+
+	val = readl(mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
+	val = val &
+		~(MAX_BUS_REQ_LEN_MASK << MAX_BUS_WR_REQ_BIT_POS);
+	val = val |
+		(mgmtdev->max_write_request << MAX_BUS_WR_REQ_BIT_POS);
+	val = val & ~(MAX_BUS_REQ_LEN_MASK);
+	val = val | (mgmtdev->max_read_request);
+	writel(val, mgmtdev->dev_virtaddr + MAX_BUS_REQ_LEN_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
+	val = val &
+		~(MAX_WR_XACTIONS_MASK << MAX_WR_XACTIONS_BIT_POS);
+	val = val |
+		(mgmtdev->max_wr_xactions << MAX_WR_XACTIONS_BIT_POS);
+	val = val & ~(MAX_RD_XACTIONS_MASK);
+	val = val | (mgmtdev->max_rd_xactions);
+	writel(val, mgmtdev->dev_virtaddr + MAX_XACTIONS_OFFSET);
+
+	mgmtdev->sw_version = readl(mgmtdev->dev_virtaddr + SW_VERSION_OFFSET);
+
+	for (i = 0; i < mgmtdev->dma_channels; i++) {
+		val = readl(mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
+		val = val & ~(1 << PRIORITY_BIT_POS);
+		val = val |
+			((mgmtdev->priority[i] & 0x1) << PRIORITY_BIT_POS);
+		val = val & ~(WEIGHT_MASK << WRR_BIT_POS);
+		val = val
+			| ((mgmtdev->weight[i] & WEIGHT_MASK) << WRR_BIT_POS);
+		writel(val, mgmtdev->dev_virtaddr + QOS_N_OFFSET + (4 * i));
+	}
+
+	val = readl(mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
+	val = val & ~CHRESET_TIMEOUUT_MASK;
+	val = val | (mgmtdev->chreset_timeout & CHRESET_TIMEOUUT_MASK);
+	writel(val, mgmtdev->dev_virtaddr + CHRESET_TIMEOUT_OFFSET);
+
+	val = readl(mgmtdev->dev_virtaddr + CFG_OFFSET);
+	val = val | 1;
+	writel(val, mgmtdev->dev_virtaddr + CFG_OFFSET);
+
+	return 0;
+}
+
+static int qcom_hidma_mgmt_probe(struct platform_device *pdev)
+{
+	struct resource *dma_resource;
+	int irq;
+	int rc;
+	u32 i;
+	struct qcom_hidma_mgmt_dev *mgmtdev;
+
+	pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	dma_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!dma_resource) {
+		dev_err(&pdev->dev, "No memory resources found\n");
+		rc = -ENODEV;
+		goto out;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "irq resources not found\n");
+		rc = -ENODEV;
+		goto out;
+	}
+
+	mgmtdev = devm_kzalloc(&pdev->dev, sizeof(*mgmtdev), GFP_KERNEL);
+	if (!mgmtdev) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	mgmtdev->pdev = pdev;
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	mgmtdev->dev_addrsize = resource_size(dma_resource);
+	mgmtdev->dev_virtaddr = devm_ioremap_resource(&pdev->dev,
+							dma_resource);
+	if (IS_ERR(mgmtdev->dev_virtaddr)) {
+		dev_err(&pdev->dev, "can't map i/o memory at %pa\n",
+			&dma_resource->start);
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "dma-channels",
+		&mgmtdev->dma_channels)) {
+		dev_err(&pdev->dev, "number of channels missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "channel-reset-timeout",
+		&mgmtdev->chreset_timeout)) {
+		dev_err(&pdev->dev, "channel reset timeout missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "max-write-burst-bytes",
+		&mgmtdev->max_write_request)) {
+		dev_err(&pdev->dev, "max-write-burst-bytes missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+	if ((mgmtdev->max_write_request != 128) &&
+		(mgmtdev->max_write_request != 256) &&
+		(mgmtdev->max_write_request != 512) &&
+		(mgmtdev->max_write_request != 1024)) {
+		dev_err(&pdev->dev, "invalid write request %d\n",
+			mgmtdev->max_write_request);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "max-read-burst-bytes",
+		&mgmtdev->max_read_request)) {
+		dev_err(&pdev->dev, "max-read-burst-bytes missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if ((mgmtdev->max_read_request != 128) &&
+		(mgmtdev->max_read_request != 256) &&
+		(mgmtdev->max_read_request != 512) &&
+		(mgmtdev->max_read_request != 1024)) {
+		dev_err(&pdev->dev, "invalid read request %d\n",
+			mgmtdev->max_read_request);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "max-write-transactions",
+		&mgmtdev->max_wr_xactions)) {
+		dev_err(&pdev->dev, "max-write-transactions missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "max-read-transactions",
+		&mgmtdev->max_rd_xactions)) {
+		dev_err(&pdev->dev, "max-read-transactions missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	mgmtdev->priority = devm_kcalloc(&pdev->dev,
+		mgmtdev->dma_channels, sizeof(*mgmtdev->priority), GFP_KERNEL);
+	if (!mgmtdev->priority) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	mgmtdev->weight = devm_kcalloc(&pdev->dev,
+		mgmtdev->dma_channels, sizeof(*mgmtdev->weight), GFP_KERNEL);
+	if (!mgmtdev->weight) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	if (device_property_read_u32_array(&pdev->dev, "channel-priority",
+				mgmtdev->priority, mgmtdev->dma_channels)) {
+		dev_err(&pdev->dev, "channel-priority missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (device_property_read_u32_array(&pdev->dev, "channel-weight",
+				mgmtdev->weight, mgmtdev->dma_channels)) {
+		dev_err(&pdev->dev, "channel-weight missing\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	for (i = 0; i < mgmtdev->dma_channels; i++) {
+		if (mgmtdev->weight[i] > 15) {
+			dev_err(&pdev->dev,
+				"max value of weight can be 15.\n");
+			rc = -EINVAL;
+			goto out;
+		}
+
+		/* weight needs to be at least one */
+		if (mgmtdev->weight[i] == 0)
+			mgmtdev->weight[i] = 1;
+	}
+
+	rc = qcom_hidma_mgmt_setup(mgmtdev);
+	if (rc) {
+		dev_err(&pdev->dev, "setup failed\n");
+		goto out;
+	}
+
+	rc = qcom_hidma_mgmt_debug_init(mgmtdev);
+	if (rc) {
+		dev_err(&pdev->dev, "debugfs init failed\n");
+		goto out;
+	}
+
+	dev_info(&pdev->dev,
+		"HI-DMA engine management driver registration complete\n");
+	platform_set_drvdata(pdev, mgmtdev);
+	pm_runtime_mark_last_busy(&mgmtdev->pdev->dev);
+	pm_runtime_put_autosuspend(&mgmtdev->pdev->dev);
+	return 0;
+out:
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_sync_suspend(&pdev->dev);
+	return rc;
+}
+
+static int qcom_hidma_mgmt_remove(struct platform_device *pdev)
+{
+	struct qcom_hidma_mgmt_dev *mgmtdev = platform_get_drvdata(pdev);
+
+	pm_runtime_get_sync(&mgmtdev->pdev->dev);
+	qcom_hidma_mgmt_debug_uninit(mgmtdev);
+	pm_runtime_put_sync_suspend(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	dev_info(&pdev->dev, "HI-DMA engine management driver removed\n");
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id qcom_hidma_mgmt_acpi_ids[] = {
+	{"QCOM8060"},
+	{},
+};
+#endif
+
+static const struct of_device_id qcom_hidma_mgmt_match[] = {
+	{ .compatible = "qcom,hidma-mgmt-1.0", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, qcom_hidma_mgmt_match);
+
+static struct platform_driver qcom_hidma_mgmt_driver = {
+	.probe = qcom_hidma_mgmt_probe,
+	.remove = qcom_hidma_mgmt_remove,
+	.driver = {
+		.name = "hidma-mgmt",
+		.of_match_table = qcom_hidma_mgmt_match,
+		.acpi_match_table = ACPI_PTR(qcom_hidma_mgmt_acpi_ids),
+	},
+};
+module_platform_driver(qcom_hidma_mgmt_driver);
+MODULE_LICENSE("GPL v2");