On 22/08/2024 13:45, Md Sadre Alam wrote: > > > On 8/22/2024 11:57 AM, Krzysztof Kozlowski wrote: >> On 21/08/2024 18:34, Md Sadre Alam wrote: >>> >>> >>> On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote: >>>> On 15/08/2024 10:57, Md Sadre Alam wrote: >>>>> BAM having pipe locking mechanism. The Lock and Un-Lock bit >>>>> should be set on CMD descriptor only. Upon encountering a >>>>> descriptor with Lock bit set, the BAM will lock all other >>>>> pipes not related to the current pipe group, and keep >>>>> handling the current pipe only until it sees the Un-Lock >>>>> set. >>>> >>>> Please wrap commit message according to Linux coding style / submission >>>> process (neither too early nor over the limit): >>>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 >>> Ok , will update in next patch. >>>> >>>>> >>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx> >>>>> --- >>>>> >>>>> Change in [v2] >>>>> >>>>> * Added initial support for dt-binding >>>>> >>>>> Change in [v1] >>>>> >>>>> * This patch was not included in [v1] >>>>> >>>>> Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>>>> index 3ad0d9b1fbc5..91cc2942aa62 100644 >>>>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>>>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>>>> @@ -77,6 +77,12 @@ properties: >>>>> Indicates that the bam is powered up by a remote processor but must be >>>>> initialized by the local processor. >>>>> >>>>> + qcom,bam_pipe_lock: >>>> >>>> Please follow DTS coding style. >>> Ok >>>> >>>>> + type: boolean >>>>> + description: >>>>> + Indicates that the bam pipe needs locking or not based on client driver >>>>> + sending the LOCK or UNLOK bit set on command descriptor. >>>> >>>> You described the desired Linux feature or behavior, not the actual >>>> hardware. The bindings are about the latter, so instead you need to >>>> rephrase the property and its description to match actual hardware >>>> capabilities/features/configuration etc. >>> Ok, will update in next patch. >>>> >>>>> + >>>>> reg: >>>>> maxItems: 1 >>>>> >>>>> @@ -92,6 +98,8 @@ anyOf: >>>>> - qcom,powered-remotely >>>>> - required: >>>>> - qcom,controlled-remotely >>>>> + - required: >>>>> + - qcom,bam_pipe_lock >>>> >>>> Why is it here? What do you want to achieve? >>> This property added to achieve locking/unlocking >>> of BAM pipe groups for mutual exclusion of resources >>> that can be used across multiple EE's >> >> This explains me nothing. I am questioning the anyOf block. Why this is >> the fourth method of controlling BAM? Anyway, if it is, then explain >> this in commit msg. > This is the BAM property for locking/unlocking the BAM pipes.That's > why I kept in anyOf block. You keep repeating the same. It's like poking me with the same comment till I agree. I am done with this. NAK. Provide proper rationale. Best regards, Krzysztof