Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-14 Thread Sinan Kaya
Hi Mark,

On 12/14/2015 7:05 AM, Mark Rutland wrote:
>> +Required properties:
>> +- compatible: "qcom,hidma-mgmt-1.0";
>> +- reg: Address range for DMA device
>
> Does this cover just the management registers, or those for channels as
> well?

 just management.

>
>> +- dma-channels: Number of channels supported by this DMA controller.
>
> Surely this is discoverable, or can be derived from the set of channels
> described in the DT?

 No, this is a HW configuration. Each hardware instance supports certain
 number of channels based on the HW build. The number of active channels
 on the running operating system does not necessarily represent the
 maximum possible.
>>>
>>> I don't follow.
>>>
>>> If you aren't using some channels, why do you need to know about them?
>>>
>>
>> I mean the hypervisor OS may not be using the channels. It could be the
>> guest machines that are using it. I need the number of the channels not
>> the memory addresses where each channel is.
> 
> I still don't follow. Knowing the number of channels alone is clearly
> insufficient.
>
> The hypervisor is in charge of handing these out to guests. So it needs
> to know the set of of channels (i.e. the address of each channel, and
> which index the channel has in the control interface). Otherwise it
> cannot possibly allocate them to guests, and cannot possibly control
> those channels at runtime.

For clarification, let me reiterate.

Hypervisor operating system certainly knows which channels and
management interfaces there are in the system. However, the information
feeds into the OS from two different drivers.

The HW ownership has been partitioned among two drivers as management
driver and channel driver. Both of them serve the hypervisor but the
management driver doesn't need to know about the channel addresses where
the transfer descriptors are set up. Similarly, the channel driver
doesn't need to know anything about the management interface where the
priority and weight assignment is done.

> 
> Given the hypervisor is in chage of allocating channels to guests, it
> knows which channels are in use, and can decide whether or not to use
> channels for its own purposes.

Agreed

> 
>>> The driver seems to assume it can access registers for all (linearly
>>> indexed) channels up to the value it read from dt for dma-channels. That
>>> doesn't seem right if the driver is not dealing with all of those
>>> channels.
>>
>> I assume you are talking about this. Feel free to be specific if not
>> this one.
>>
>>  for (i = 0; i < mgmtdev->dma_channels; i++) {
>>  u32 weight = mgmtdev->weight[i];
>>  u32 priority = mgmtdev->priority[i];
>>
>>  val = readl(mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
>>  val &= ~(1 << HIDMA_PRIORITY_BIT_POS);
>>  val |= (priority & 0x1) << HIDMA_PRIORITY_BIT_POS;
>>  val &= ~(HIDMA_WEIGHT_MASK << HIDMA_WRR_BIT_POS);
>>  val |= (weight & HIDMA_WEIGHT_MASK) << HIDMA_WRR_BIT_POS;
>>  writel(val, mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
>>  }
>>
>> The management driver is configuring the priority and weight registers
>> inside the management address space. It is not accessing the channel
>> address space where data transfer descriptors are set up. There is one
>> register for priority & weight of the each channel inside the management
>> address space.
> 
> My point was that this implies that the driver has no idea as to the
> relationship between these configuration registers and the actual
> channel address spaces. It has no idea which channel address spaces are
> being configured here. That does not seem right.

My intention was to use the ACPI _UID to associate the channel index.
The _UID would tell me the index of the channel. I was encoding UID as
follows in the ACPI table.



this would tell me which HIDMA this channel belongs to as well as which
channel it is.

Since this solution is very much ACPI specific and doesn't scale to
device-tree, I'm thinking of adding a sysfs "index" file into each
channel so that I don't need to search through the firmware_node in ACPI
and of_node directory in device-tree mode.

I'm also thinking of creating symbolic links from the channel object
into the management object.

> hidma {
>   compatible - "qcom,hidma-1.0";
>
>   /* OPTIONAL management interface registers */
>   reg = < ... ... >;
>
>   ...
>
>   channel0 {
>   compatible = "qcom,
>   reg = < ... ... >;
>
>   ...
>   };
>
>   ...
> };
>>
>> This seems to have worked without requiring any work from me.
>>

Here is something I don't understand why and I could get some help from
DT experts.

When I have ACPI as follows

device(mgmt)
{
device(channel0)
{
}
}

The channel0 driver gets automatically probed by the OS.

Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-14 Thread Mark Rutland
>  +Required properties:
>  +- compatible: "qcom,hidma-mgmt-1.0";
>  +- reg: Address range for DMA device
> >>>
> >>> Does this cover just the management registers, or those for channels as
> >>> well?
> >>
> >> just management.
> >>
> >>>
>  +- dma-channels: Number of channels supported by this DMA controller.
> >>>
> >>> Surely this is discoverable, or can be derived from the set of channels
> >>> described in the DT?
> >>
> >> No, this is a HW configuration. Each hardware instance supports certain
> >> number of channels based on the HW build. The number of active channels
> >> on the running operating system does not necessarily represent the
> >> maximum possible.
> > 
> > I don't follow.
> > 
> > If you aren't using some channels, why do you need to know about them?
> > 
> 
> I mean the hypervisor OS may not be using the channels. It could be the
> guest machines that are using it. I need the number of the channels not
> the memory addresses where each channel is.

I still don't follow. Knowing the number of channels alone is clearly
insufficient.

The hypervisor is in charge of handing these out to guests. So it needs
to know the set of of channels (i.e. the address of each channel, and
which index the channel has in the control interface). Otherwise it
cannot possibly allocate them to guests, and cannot possibly control
those channels at runtime.

Given the hypervisor is in chage of allocating channels to guests, it
knows which channels are in use, and can decide whether or not to use
channels for its own purposes.

> > The driver seems to assume it can access registers for all (linearly
> > indexed) channels up to the value it read from dt for dma-channels. That
> > doesn't seem right if the driver is not dealing with all of those
> > channels.
> 
> I assume you are talking about this. Feel free to be specific if not
> this one.
> 
>   for (i = 0; i < mgmtdev->dma_channels; i++) {
>   u32 weight = mgmtdev->weight[i];
>   u32 priority = mgmtdev->priority[i];
> 
>   val = readl(mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
>   val &= ~(1 << HIDMA_PRIORITY_BIT_POS);
>   val |= (priority & 0x1) << HIDMA_PRIORITY_BIT_POS;
>   val &= ~(HIDMA_WEIGHT_MASK << HIDMA_WRR_BIT_POS);
>   val |= (weight & HIDMA_WEIGHT_MASK) << HIDMA_WRR_BIT_POS;
>   writel(val, mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
>   }
> 
> The management driver is configuring the priority and weight registers
> inside the management address space. It is not accessing the channel
> address space where data transfer descriptors are set up. There is one
> register for priority & weight of the each channel inside the management
> address space.

My point was that this implies that the driver has no idea as to the
relationship between these configuration registers and the actual
channel address spaces. It has no idea which channel address spaces are
being configured here. That does not seem right.

> >>> Why can this information not be associated with the channel directly?
> >>>
> >> Two reasons:
> >> 1. The channel doesn't have the capability to change the priority and
> >> weight. HW design. Management HW can do this only.
> > 
> > You can describe the channels directly to the hypervisor which talks to
> > the management HW.
> 
> Not a use case, I don't want to allow guest machine to change their
> priority and weight.

I didn't suggest that the guest would. Regardless, it seems these should
be runtime configured anyway, at which point you need to know the
relationship.

> >>> There's no relationship to channels defined here. What happens if/when
> >>> you have a system with multiple instances?
> >>>
> >>
> >> I do support multiple instances. I tested with 4 instances (6 channels
> >> each). This driver is only responsible for management which it can do
> >> through its own dedicated HW interface. It doesn't need access to the
> >> channel address space. There will be 4 HIDMA management instances in
> >> this case.
> > 
> > I don't follow.
> > 
> > How do you know which channels are associated with which management
> > interface?
> 
> The association is the other way around. User need to know the
> management interface for a channel rather than the channel needing to
> know the management interface.
> 
> I'll think about it.

Ok. I agree that's the way the user will look at it.

> > How can your sysfs interface work if that relationship is not described?
> 
> Here is the sysfs documentation. Each management object has one channel
> directory under it. I still don't need to access the channel object in
> order to change its weight and priority.
> 
> What:   /sys/devices/platform/hidma-mgmt*/chan*/priority
> /sys/devices/platform/QCOM8060:*/chan*/priority
> Date:   Nov 2015
> KernelVersion:  4.4
> Contact:"Sinan Kaya "
> Description:
> Contains either 0 or 

Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-11 Thread Sinan Kaya
On 12/11/2015 2:37 PM, Mark Rutland wrote:
>>
>> Another reviewer requested guidance on how to set these parameters.
>> That's why, I tried to provide as much data as possible.
> 
> It's perfectly fine to have some other document for the driver, but the
> binding doc should stick to HW description.
> 

OK, I'll reword and put some more HW description.

 +Required properties:
 +- compatible: "qcom,hidma-mgmt-1.0";
 +- reg: Address range for DMA device
>>>
>>> Does this cover just the management registers, or those for channels as
>>> well?
>>
>> just management.
>>
>>>
 +- dma-channels: Number of channels supported by this DMA controller.
>>>
>>> Surely this is discoverable, or can be derived from the set of channels
>>> described in the DT?
>>
>> No, this is a HW configuration. Each hardware instance supports certain
>> number of channels based on the HW build. The number of active channels
>> on the running operating system does not necessarily represent the
>> maximum possible.
> 
> I don't follow.
> 
> If you aren't using some channels, why do you need to know about them?
> 

I mean the hypervisor OS may not be using the channels. It could be the
guest machines that are using it. I need the number of the channels not
the memory addresses where each channel is.

> The driver seems to assume it can access registers for all (linearly
> indexed) channels up to the value it read from dt for dma-channels. That
> doesn't seem right if the driver is not dealing with all of those
> channels.

I assume you are talking about this. Feel free to be specific if not
this one.

for (i = 0; i < mgmtdev->dma_channels; i++) {
u32 weight = mgmtdev->weight[i];
u32 priority = mgmtdev->priority[i];

val = readl(mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
val &= ~(1 << HIDMA_PRIORITY_BIT_POS);
val |= (priority & 0x1) << HIDMA_PRIORITY_BIT_POS;
val &= ~(HIDMA_WEIGHT_MASK << HIDMA_WRR_BIT_POS);
val |= (weight & HIDMA_WEIGHT_MASK) << HIDMA_WRR_BIT_POS;
writel(val, mgmtdev->virtaddr + HIDMA_QOS_N_OFFSET + (4 * i));
}

The management driver is configuring the priority and weight registers
inside the management address space. It is not accessing the channel
address space where data transfer descriptors are set up. There is one
register for priority & weight of the each channel inside the management
address space.

> 
>>>
>>> Why can this information not be associated with the channel directly?
>>>
>> Two reasons:
>> 1. The channel doesn't have the capability to change the priority and
>> weight. HW design. Management HW can do this only.
> 
> You can describe the channels directly to the hypervisor which talks to
> the management HW.

Not a use case, I don't want to allow guest machine to change their
priority and weight. Guest machine runs the channel driver.

> 
>> 2. We are building SW to change the channel priority and weight at
>> runtime from the hypervisor through sysfs. The system administrator of
>> the server will reallocate resources based on the guest machine
>> requirements.
> 
> Ok, so these properties are not necessary at all.
> 
> They can go, and you can default to a reasonable fair QoS configuration
> that the user can override at runtime depending on use-case.

Partially agreed, some values are "the fair QoS configuration defaults"
per platform like maximum-read-request etc. The firmware writes the
default values in their device tree/ ACPI table for HW description and
the HIDMA driver will configure the defaults.

I'll remove the weight and priority though.

> 
>>> How does one choose the right priority and/or weight? These seem like
>>> runtime details given that channels are indned to be allocated by
>>> software.
>>
>> priority = 0..1
>> weight = 0...15 (adding max value to the documentation)
> 
> My point was more that the value you want depends on your use-case,
> which is _not_ a static hardware property, but rather a dynamic run-time
> property.
> 
> You stated this can be modified at run time. It should not be in the DT.
> Please drop it from the binding.
> 

I'll get rid of weight and priority only.

>> +  Valid values are 1..15.
>>
>>>
>>> There's no relationship to channels defined here. What happens if/when
>>> you have a system with multiple instances?
>>>
>>
>> I do support multiple instances. I tested with 4 instances (6 channels
>> each). This driver is only responsible for management which it can do
>> through its own dedicated HW interface. It doesn't need access to the
>> channel address space. There will be 4 HIDMA management instances in
>> this case.
> 
> I don't follow.
> 
> How do you know which channels are associated with which management
> interface?

The association is the other way around. User need to know the
management interface for a channel rather than the channel needing to
know the management interface.

I'll think

Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-11 Thread Mark Rutland
> >> 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 000..b632635
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> >> @@ -0,0 +1,61 @@
> >> +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.
> >> +
> >> +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
> >> +behavior dynamically such as bandwidth allocation and
> >> +prioritization.
> >> +
> >> +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.
> > 
> > This is a mixture of hardware and software description (e.g. VFIO has
> > nothing to do wit hteh hardware). We want to capture what is necessary
> > to describe the hardware, not what the software stack above it will look
> > like.
> 
> Another reviewer requested guidance on how to set these parameters.
> That's why, I tried to provide as much data as possible.

It's perfectly fine to have some other document for the driver, but the
binding doc should stick to HW description.

> >> +Required properties:
> >> +- compatible: "qcom,hidma-mgmt-1.0";
> >> +- reg: Address range for DMA device
> > 
> > Does this cover just the management registers, or those for channels as
> > well?
> 
> just management.
> 
> > 
> >> +- dma-channels: Number of channels supported by this DMA controller.
> > 
> > Surely this is discoverable, or can be derived from the set of channels
> > described in the DT?
> 
> No, this is a HW configuration. Each hardware instance supports certain
> number of channels based on the HW build. The number of active channels
> on the running operating system does not necessarily represent the
> maximum possible.

I don't follow.

If you aren't using some channels, why do you need to know about them?

The driver seems to assume it can access registers for all (linearly
indexed) channels up to the value it read from dt for dma-channels. That
doesn't seem right if the driver is not dealing with all of those
channels.

> >> +- 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-cycles: Channel reset timeout in cycles 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.
> > 
> > Per the example, these last two seem to be arrays, which wasn't clear
> > from the description.
> 
> OK, let me clarify this. New text:
> +- channel-priority: an array of channel priorities.
> +- channel-weight: an array of round robin channel weights
> 
> > 
> > Why can this information not be associated with the channel directly?
> > 
> Two reasons:
> 1. The channel doesn't have the capability to change the priority and
> weight. HW design. Management HW can do this only.

You can describe the channels directly to the hypervisor which talks to
the management HW.

> 2. We are building SW to change the channel priority and weight at
> runtime from the hypervisor through sysfs. The system administrator of
> the server will reallocate resources based on the guest machine
> requirements.

Ok, so these properties are not necessary at all.

They can go, and you can default to a reasonable fair QoS configuration
that 

Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-11 Thread Sinan Kaya
Hi Mark,

On 12/11/2015 11:41 AM, Mark Rutland wrote:
> Hi,
> 
> On Fri, Dec 11, 2015 at 11:16:58AM -0500, Sinan Kaya 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.
>>
>> 1. HIDMA Management driver
>> 2. HIDMA Channel driver
>>
>> 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
>> behavior dynamically such as bandwidth allocation and prioritization.
>>
>> The management driver is executed in hypervisor context and is the main
>> management entity for all channels provided by the device.
>>
>> Signed-off-by: Sinan Kaya 
>> Reviewed-by: Andy Shevchenko 
>> ---
>>  .../ABI/testing/sysfs-platform-hidma-mgmt  |  97 +++
>>  .../devicetree/bindings/dma/qcom_hidma_mgmt.txt|  61 
> 
> Please split the binding into a separate patch, per
> Documentation/devicetree/bindings/submitting-patches.txt.

Done. I'm new to this. Bare with me.

> 
>> 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 000..b632635
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
>> @@ -0,0 +1,61 @@
>> +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.
>> +
>> +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
>> +behavior dynamically such as bandwidth allocation and
>> +prioritization.
>> +
>> +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.
> 
> This is a mixture of hardware and software description (e.g. VFIO has
> nothing to do wit hteh hardware). We want to capture what is necessary
> to describe the hardware, not what the software stack above it will look
> like.

Another reviewer requested guidance on how to set these parameters.
That's why, I tried to provide as much data as possible.

> 
>> +Required properties:
>> +- compatible: "qcom,hidma-mgmt-1.0";
>> +- reg: Address range for DMA device
> 
> Does this cover just the management registers, or those for channels as
> well?

just management.

> 
>> +- dma-channels: Number of channels supported by this DMA controller.
> 
> Surely this is discoverable, or can be derived from the set of channels
> described in the DT?

No, this is a HW configuration. Each hardware instance supports certain
number of channels based on the HW build. The number of active channels
on the running operating system does not necessarily represent the
maximum possible.

> 
>> +- 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-cycles: Channel reset timeout in cycles 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.
> 
> Per the example, these last two seem to be arrays, which wasn't clear
> from the description.

OK, let me clarify this. New text:
+- channel-priority: an array of channel priorities.
+- channel-weight: an array of round robin channel weights

> 
> Why can th

Re: [PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-11 Thread Mark Rutland
Hi,

On Fri, Dec 11, 2015 at 11:16:58AM -0500, Sinan Kaya 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.
> 
> 1. HIDMA Management driver
> 2. HIDMA Channel driver
> 
> 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
> behavior dynamically such as bandwidth allocation and prioritization.
> 
> The management driver is executed in hypervisor context and is the main
> management entity for all channels provided by the device.
> 
> Signed-off-by: Sinan Kaya 
> Reviewed-by: Andy Shevchenko 
> ---
>  .../ABI/testing/sysfs-platform-hidma-mgmt  |  97 +++
>  .../devicetree/bindings/dma/qcom_hidma_mgmt.txt|  61 

Please split the binding into a separate patch, per
Documentation/devicetree/bindings/submitting-patches.txt.

> 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 000..b632635
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> @@ -0,0 +1,61 @@
> +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.
> +
> +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
> +behavior dynamically such as bandwidth allocation and
> +prioritization.
> +
> +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.

This is a mixture of hardware and software description (e.g. VFIO has
nothing to do wit hteh hardware). We want to capture what is necessary
to describe the hardware, not what the software stack above it will look
like.

> +Required properties:
> +- compatible: "qcom,hidma-mgmt-1.0";
> +- reg: Address range for DMA device

Does this cover just the management registers, or those for channels as
well?

> +- dma-channels: Number of channels supported by this DMA controller.

Surely this is discoverable, or can be derived from the set of channels
described in the DT?

> +- 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-cycles: Channel reset timeout in cycles 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.

Per the example, these last two seem to be arrays, which wasn't clear
from the description.

Why can this information not be associated with the channel directly?

How does one choose the right priority and/or weight? These seem like
runtime details given that channels are indned to be allocated by
software.

There's no relationship to channels defined here. What happens if/when
you have a system with multiple instances?

> +
> +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-cycles = 0x500;

Ple

[PATCH V9 2/5] dma: add Qualcomm Technologies HIDMA management driver

2015-12-11 Thread Sinan Kaya
The Qualcomm Technologies HIDMA device has been designed to support
virtualization technology. The driver has been divided into two to follow
the hardware design.

1. HIDMA Management driver
2. HIDMA Channel driver

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
behavior dynamically such as bandwidth allocation and prioritization.

The management driver is executed in hypervisor context and is the main
management entity for all channels provided by the device.

Signed-off-by: Sinan Kaya 
Reviewed-by: Andy Shevchenko 
---
 .../ABI/testing/sysfs-platform-hidma-mgmt  |  97 +++
 .../devicetree/bindings/dma/qcom_hidma_mgmt.txt|  61 
 drivers/dma/qcom/Kconfig   |  11 +
 drivers/dma/qcom/Makefile  |   1 +
 drivers/dma/qcom/hidma_mgmt.c  | 318 +
 drivers/dma/qcom/hidma_mgmt.h  |  39 +++
 drivers/dma/qcom/hidma_mgmt_sys.c  | 292 +++
 7 files changed, 819 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-hidma-mgmt
 create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
 create mode 100644 drivers/dma/qcom/hidma_mgmt.c
 create mode 100644 drivers/dma/qcom/hidma_mgmt.h
 create mode 100644 drivers/dma/qcom/hidma_mgmt_sys.c

diff --git a/Documentation/ABI/testing/sysfs-platform-hidma-mgmt 
b/Documentation/ABI/testing/sysfs-platform-hidma-mgmt
new file mode 100644
index 000..4fc6876
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-hidma-mgmt
@@ -0,0 +1,97 @@
+What:  /sys/devices/platform/hidma-mgmt*/chan*/priority
+   /sys/devices/platform/QCOM8060:*/chan*/priority
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Contains either 0 or 1 and indicates if the DMA channel is a
+   low priority (0) or high priority (1) channel.
+
+What:  /sys/devices/platform/hidma-mgmt*/chan*/weight
+   /sys/devices/platform/QCOM8060:*/chan*/weight
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Contains 0..15 and indicates the weight of the channel among
+   equal priority channels during round robin scheduling.
+
+What:  /sys/devices/platform/hidma-mgmt*/chreset_timeout_cycles
+   /sys/devices/platform/QCOM8060:*/chreset_timeout_cycles
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Contains the platform specific cycle value to wait after a
+   reset command is issued. If the value is chosen too short,
+   then the HW will issue a reset failure interrupt. The value
+   is platform specific and should not be changed without
+   consultance.
+
+What:  /sys/devices/platform/hidma-mgmt*/dma_channels
+   /sys/devices/platform/QCOM8060:*/dma_channels
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Contains the number of dma channels supported by one instance
+   of HIDMA hardware. The value may change from chip to chip.
+
+What:  /sys/devices/platform/hidma-mgmt*/hw_version_major
+   /sys/devices/platform/QCOM8060:*/hw_version_major
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Version number major for the hardware.
+
+What:  /sys/devices/platform/hidma-mgmt*/hw_version_minor
+   /sys/devices/platform/QCOM8060:*/hw_version_minor
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Version number minor for the hardware.
+
+What:  /sys/devices/platform/hidma-mgmt*/max_rd_xactions
+   /sys/devices/platform/QCOM8060:*/max_rd_xactions
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Contains a value between 0 and 31. Maximum number of
+   read transactions that can be issued back to back.
+   Choosing a higher number gives better performance but
+   can also cause performance reduction to other peripherals
+   sharing the same bus.
+
+What:  /sys/devices/platform/hidma-mgmt*/max_read_request
+   /sys/devices/platform/QCOM8060:*/max_read_request
+Date:  Nov 2015
+KernelVersion: 4.4
+Contact:   "Sinan Kaya "
+Description:
+   Size of each read request. The value needs to be a power
+   of two and can be between 128 and 1024.
+
+What:  /sys/devices/platform/hidma-mgmt*/max_wr