Re: [PATCH 1/3] dt-bindings: interconnect: Add Qualcomm QCS404 DT bindings

[Georgi Djakov]

Hi Bjorn,

On 4/5/19 21:32, Bjorn Andersson wrote:
> On Fri 05 Apr 10:54 +07 2019, Georgi Djakov wrote:
> 
>> The Qualcomm QCS404 platform has several buses that could be controlled
>> and tuned according to the bandwidth demand.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
>> ---
>>  .../bindings/interconnect/qcom,qcs404.txt     | 45 +++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt b/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>> new file mode 100644
>> index 000000000000..2ea63ea827d7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>> @@ -0,0 +1,45 @@
>> +Qualcomm QCS404 Network-On-Chip interconnect driver binding
>> +-----------------------------------------------------------
>> +
>> +Required properties :
>> +- compatible : shall contain only one of the following:
>> +			"qcom,qcs404-bimc"
> 
> As this is a hardware block available in mmio register space I think you
> better represent this on the mmio (soc) bus - and then represent the
> link to rpm as a child node of the rpm.

The mmio register space is not used for expressing bandwidth needs, but
contains mostly QoS related stuff. We do not support QoS for now and
that's why i haven't included it. When we decide to support QoS, we can
add the nodes for the QoS registers and then reference them with some DT
property like qcom,qos = <&bimc_qos>;

Thanks,
Georgi

> 
> Apart from that this looks good.
> 
> Regards,
> Bjorn
> 
>> +			"qcom,qcs404-pcnoc"
>> +			"qcom,qcs404-snoc"
>> +- #interconnect-cells : should contain 1
>> +
>> +Optional properties :
>> +clocks : list of phandles and specifiers to all interconnect bus clocks
>> +clock-names : clock names should include both "bus_clk" and "bus_a_clk"
>> +
>> +Example:
>> +
>> +rpm-glink {
>> +	...
>> +	rpm_requests: glink-channel {
>> +		...
>> +		bimc: interconnect@0 {
>> +			compatible = "qcom,qcs404-bimc";
>> +			#interconnect-cells = <1>;
>> +			clock-names = "bus_clk", "bus_a_clk";
>> +			clocks = <&rpmcc RPM_SMD_BIMC_CLK>,
>> +				<&rpmcc RPM_SMD_BIMC_A_CLK>;
>> +		};
>> +
>> +		pnoc: interconnect@1 {
>> +			compatible = "qcom,qcs404-pcnoc";
>> +			#interconnect-cells = <1>;
>> +			clock-names = "bus_clk", "bus_a_clk";
>> +			clocks = <&rpmcc RPM_SMD_PNOC_CLK>,
>> +				<&rpmcc RPM_SMD_PNOC_A_CLK>;
>> +		};
>> +
>> +		snoc: interconnect@2 {
>> +			compatible = "qcom,qcs404-snoc";
>> +			#interconnect-cells = <1>;
>> +			clock-names = "bus_clk", "bus_a_clk";
>> +			clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
>> +				<&rpmcc RPM_SMD_SNOC_A_CLK>;
>> +		};
>> +	};
>> +};