linux-kernel - Re: [PATCH] dmaengine: qcom_bam_dma: add one more optional clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 08 Sep 2014 18:43:58 +0300
From:	Stanimir Varbanov <svarbanov@...sol.com>
To:	Mark Rutland <mark.rutland@....com>,
	Andy Gross <agross@...eaurora.org>
CC:	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Pawel Moll <Pawel.Moll@....com>,
	Rob Herring <robh+dt@...nel.org>,
	Kumar Gala <galak@...eaurora.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH] dmaengine: qcom_bam_dma: add one more optional clock

Hi Mark,

Thank you for the comments!

On 09/08/2014 01:36 PM, Mark Rutland wrote:
> On Sun, Sep 07, 2014 at 06:55:47PM +0100, Stanimir Varbanov wrote:
>> The BAM is tightly coupled with the peripheral to which it
>> belongs. In that sprit to access the BAM configuration
>> registers the driver needs to enable some peripheral
>> clocks. Currently the DT node enables bamclk which seems
>> is not enough for some peripherals (for example the crypto
>> engine wants core and iface clocks). This change attempts
>> to solve this issue by adding one more optional clock
>> in bam_dma driver.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@...sol.com>
>> ---
>>  .../devicetree/bindings/dma/qcom_bam_dma.txt       | 12 ++++--
>>  drivers/dma/qcom_bam_dma.c                         | 44 +++++++++++++++-------
>>  2 files changed, 38 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> index d75a9d7..2376897 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> @@ -6,8 +6,11 @@ Required properties:
>>  - interrupts: Should contain the one interrupt shared by all channels
>>  - #dma-cells: must be <1>, the cell in the dmas property of the client device
>>    represents the channel number
>> -- clocks: required clock
>> -- clock-names: must contain "bam_clk" entry
>> +- clocks: list of required clock plus one optional clock. The optional clock
>> +          is needed for some peripherals and can be omitted.
>> +- clock-names: must contain "core" clock name representing the required clock
>> +               plus the optional "iface" clock name depending on
>> +               peripheral needs.
> 
> Please don't change the names of input lines (at least without retaining
> support for the old name). Does this change not break existing DTBs?

It shouldn't break any DTBs. I changed the "bam_clk" because it sounds
like the BAM needs only one "bam_clk". But the practice shows that in
the crypto engine case I need to pass "core" and "iface" clocks to get
bam dma driver initialised.

I hope Andy can share his opinion on that, or even suggest something better.

> 
> Which of these new names did "bam_clk" previously correspond to?

The required one i.e. "core".

> 
> How many clock inputs does the BAM actually have? Are there more which
> we don't yet describe?
> 
> You mention that the iface clock is actually fed into the peripheral the
> BAM is attached to. Can you explain why this clock is necessary to use
> the BAM?

No, I can't explain why, might be hardware reasons.

> 
> If you want do describe multiple clocks and names are required, define
> clocks in terms of clock-names so you don't need to state everything
> twice, Put each clock on a new line, e.g.
> 
> - clocks: A list of phandle + clock-specifier pairs, one for each entry
>   in clock-names
> - clock-names: should contain:
>    * "foo_clk" for the FOO clock
>    * "bar_clk" for the BAR input on baz_xxxx systems

OK. Thanks.

-- 
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/