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:

quoted

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 <redacted>
---
 .../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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html