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@xxxxxxxxxx> >> --- >> .../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 dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
