linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sinan Kaya <okaya@codeaurora.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: dmaengine@vger.kernel.org, timur@codeaurora.org,
	devicetree@vger.kernel.org, cov@codeaurora.org,
	vinod.koul@intel.com, jcm@redhat.com, agross@codeaurora.org,
	arnd@arndb.de, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V12 2/7] dma: hidma: Add Device Tree support
Date: Mon, 18 Jan 2016 09:04:30 -0500	[thread overview]
Message-ID: <569CF0EE.7010706@codeaurora.org> (raw)
In-Reply-To: <20160118114931.GJ21067@leverpostej>

On 1/18/2016 6:49 AM, Mark Rutland wrote:
>>>> +Main node required properties:
>>>> +- compatible: "qcom,hidma-mgmt-1.0";
>>>> +- 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
>>>
>>> Just to check, where do these max-* values come from?
>> These are HW bus parameters like the burst count and 
>> size of each burst. These values change based on the SoC this IP is in use.
>>
>>>
>>> Are they some correctness requirement of the bus this is attached to?
>> You can starve other peripherals if you use incorrect values as the bus is 
>> shared with other peripherals. Yes, correctness is required.
> 
> Is that a property of the system known statically, or one determined by
> testing the system under particular workloads? It feels like the latter
> (though I appreciate that not starving other masters is certainly a
> correctness property regardless of how this is derived).
> 

It is known statically and is determined through simulations according to the SoC design.
It is later verified on chip. Once verified, these values never change. 
Values change from one SoC to another. 

> I'd have expected the bus this is plugged into to have appropriate QoS
> settings pre-configured so as to avoid starvation, though it sounds like
> that's not possible here?

There are some very safe values but they are not correct until validated on the chip.

> 
>>> Are they tuning values?
>> Correct value is necessary for functioning. I'd consider weight and priority
>> as the only tuning parameters. 
>>
>>>
>>> The latter doesn't really belong in the DT. Given they're writeable from
>>> the driver, it seems like that's what they are...
>>
>> Good catch. Those should have been read-only. I wanted to be able to export these
>> information to the userspace app. I'll fix the sysfs to make them read-only.
>>
>>>
>>>> +- channel-reset-timeout-cycles: Channel reset timeout in cycles for this SOC.
>>>
>>> I'm not sure what this means. Could you elaborate on this is?
>> After each reset command, HW starts a timer. This is the time HW waits before it declares
>> reset failed.
> 
> Is that a reset command sent to the HIDMA by the OS, or a reset command
> from the HIDMA to something else?

First one. 

As I said on another email, The HIDMA channel driver resets the transfer and event channels 
before using them. The reset command is intended for the HIDMA HW itself.

> 
> What does it do when it declares a reset as failed?
> 

The channel probe will fail with reset failed message.

> How can the OS make use of this information? It has no idea of the
> clocks input to the HIDMA, so it has no idea how long a cycle is.
> 
This is a HW parameter. The OS doesn't use timers or any other facility to time correctness. 

The HIDMA channel driver issues a reset and then waits for confirmation while polling a status
register. If no confirmation is received, then the HW is assumed broken or incorrectly configured.
The HW channel is removed from the OS access.

> Is this programmed by the OS?
The HIDMA management driver programs this value to the HW. It is not consumed by the OS directly.

> 
> Is the particular duration in cycles a requirement of some other agent?
It is a requirement of the HW. Not the OS.

> 
> Thanks,
> Mark.
> 


-- 
Sinan Kaya
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

  reply	other threads:[~2016-01-18 14:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11 14:45 [PATCH v12 0/7] dma: add Qualcomm Technologies HIDMA driver Sinan Kaya
2016-01-11 14:45 ` [PATCH V12 1/7] dma: qcom_bam_dma: move to qcom directory Sinan Kaya
2016-01-11 14:45 ` [PATCH V12 2/7] dma: hidma: Add Device Tree support Sinan Kaya
2016-01-15 15:16   ` Mark Rutland
2016-01-15 15:30     ` Mark Rutland
2016-01-15 17:05       ` Sinan Kaya
2016-01-18 11:39         ` Mark Rutland
2016-01-15 16:49     ` Sinan Kaya
2016-01-18 11:49       ` Mark Rutland
2016-01-18 14:04         ` Sinan Kaya [this message]
2016-01-11 14:45 ` [PATCH V12 3/7] dma: add Qualcomm Technologies HIDMA management driver Sinan Kaya
2016-01-15 14:56   ` Mark Rutland
2016-01-15 15:12     ` Sinan Kaya
2016-01-15 15:22       ` Mark Rutland
2016-01-15 17:16         ` Sinan Kaya
2016-01-15 17:32           ` Marc Zyngier
2016-01-15 22:47             ` Sinan Kaya
2016-01-18  9:06               ` Marc Zyngier
2016-01-22 18:38         ` Sinan Kaya
2016-01-15 15:14     ` Marc Zyngier
2016-01-15 15:36       ` Mark Rutland
2016-01-15 16:01         ` Sinan Kaya
2016-01-20 22:18           ` Sinan Kaya
2016-01-15 15:40       ` Sinan Kaya
2016-01-15 17:28         ` Marc Zyngier
2016-01-15 17:44           ` Sinan Kaya
2016-01-15 18:08             ` Marc Zyngier
2016-01-11 14:45 ` [PATCH V12 4/7] dma: add Qualcomm Technologies HIDMA channel driver Sinan Kaya
2016-01-11 14:45 ` [PATCH V12 5/7] dma: qcom_hidma: implement lower level hardware interface Sinan Kaya
2016-01-11 14:45 ` [PATCH V12 6/7] dma: qcom_hidma: add debugfs hooks Sinan Kaya
2016-01-11 14:45 ` [PATCH V12 7/7] dma: qcom_hidma: add support for object hierarchy Sinan Kaya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=569CF0EE.7010706@codeaurora.org \
    --to=okaya@codeaurora.org \
    --cc=agross@codeaurora.org \
    --cc=arnd@arndb.de \
    --cc=cov@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=jcm@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=timur@codeaurora.org \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).