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

[Georgi Djakov]

Hi Bjorn,

On 7/8/19 22:26, Bjorn Andersson wrote:
> On Thu 13 Jun 08:13 PDT 2019, Georgi Djakov wrote:
> 
>> The Qualcomm QCS404 platform has several buses that could be controlled
>> and tuned according to the bandwidth demand.
>>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>> Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
>> ---
>>
>> v4:
>> - Add the DT header into this patch.
>> - Pick Bjorn's r-b.
>>
>> v3:
>> - Add a reg property and move the interconnect nodes under the "soc" node.
>>
>> v2:
>> - No changes.
>>
>>  .../bindings/interconnect/qcom,qcs404.txt     | 46 ++++++++++
>>  .../dt-bindings/interconnect/qcom,qcs404.h    | 88 +++++++++++++++++++
>>  2 files changed, 134 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>>  create mode 100644 include/dt-bindings/interconnect/qcom,qcs404.h
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt b/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>> new file mode 100644
>> index 000000000000..14a827268dda
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs404.txt
>> @@ -0,0 +1,46 @@
>> +Qualcomm QCS404 Network-On-Chip interconnect driver binding
>> +-----------------------------------------------------------
>> +
>> +Required properties :
>> +- compatible : shall contain only one of the following:
>> +			"qcom,qcs404-bimc"
>> +			"qcom,qcs404-pcnoc"
>> +			"qcom,qcs404-snoc"
>> +- #interconnect-cells : should contain 1
>> +
>> +Optional properties :
>> +reg : specifies the physical base address and size of registers
>> +clocks : list of phandles and specifiers to all interconnect bus clocks
>> +clock-names : clock names should include both "bus_clk" and "bus_a_clk"
> 
> Spoke to Rob about this patch, and I don't think these properties should
> not be described as optional.
> 
> The reg isn't used unless we're implementing support for QoS, but let's
> include them in the binding as required anyways.

Ok, will do it!

> 
> Iirc the two clocks are required with the current implementation, but
> shouldn't there be an iface clock as well, for accessing the QoS
> register space?

Actually i expect this to be a list of AXI clocks of the IP blocks, whose QoS
ports we configure. For this platform, according to downstream, it would be just
<&gcc GCC_SYS_NOC_USB3_CLK>. For other platforms i see these are a few usb/ufs
clocks that should be enabled while we set QoS for usb/ufs.

> 
> PS. As I read this again, please drop _clk from the two clocks names -
> we know they are clocks...

Sure, will do it!

Thanks,
Georgi

> 
> Regards,
> Bjorn
> 
>> +
>> +Example:
>> +
>> +soc {
>> +	...
>> +	bimc: interconnect@400000 {
>> +		reg = <0x00400000 0x80000>;
>> +		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@500000 {
>> +		reg = <0x00500000 0x15080>;
>> +		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@580000 {
>> +		reg = <0x00580000 0x23080>;
>> +		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>;
>> +	};
>> +};
>> diff --git a/include/dt-bindings/interconnect/qcom,qcs404.h b/include/dt-bindings/interconnect/qcom,qcs404.h
>> new file mode 100644
>> index 000000000000..960f6e39c5f2
>> --- /dev/null
>> +++ b/include/dt-bindings/interconnect/qcom,qcs404.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Qualcomm interconnect IDs
>> + *
>> + * Copyright (c) 2019, Linaro Ltd.
>> + * Author: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
>> + */
>> +
>> +#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_QCS404_H
>> +#define __DT_BINDINGS_INTERCONNECT_QCOM_QCS404_H
>> +
>> +#define MASTER_AMPSS_M0			0
>> +#define MASTER_OXILI			1
>> +#define MASTER_MDP_PORT0		2
>> +#define MASTER_SNOC_BIMC_1		3
>> +#define MASTER_TCU_0			4
>> +#define SLAVE_EBI_CH0			5
>> +#define SLAVE_BIMC_SNOC			6
>> +
>> +#define MASTER_SPDM			0
>> +#define MASTER_BLSP_1			1
>> +#define MASTER_BLSP_2			2
>> +#define MASTER_XI_USB_HS1		3
>> +#define MASTER_CRYPT0			4
>> +#define MASTER_SDCC_1			5
>> +#define MASTER_SDCC_2			6
>> +#define MASTER_SNOC_PCNOC		7
>> +#define MASTER_QPIC			8
>> +#define PCNOC_INT_0			9
>> +#define PCNOC_INT_2			10
>> +#define PCNOC_INT_3			11
>> +#define PCNOC_S_0			12
>> +#define PCNOC_S_1			13
>> +#define PCNOC_S_2			14
>> +#define PCNOC_S_3			15
>> +#define PCNOC_S_4			16
>> +#define PCNOC_S_6			17
>> +#define PCNOC_S_7			18
>> +#define PCNOC_S_8			19
>> +#define PCNOC_S_9			20
>> +#define PCNOC_S_10			21
>> +#define PCNOC_S_11			22
>> +#define SLAVE_SPDM			23
>> +#define SLAVE_PDM			24
>> +#define SLAVE_PRNG			25
>> +#define SLAVE_TCSR			26
>> +#define SLAVE_SNOC_CFG			27
>> +#define SLAVE_MESSAGE_RAM		28
>> +#define SLAVE_DISP_SS_CFG		29
>> +#define SLAVE_GPU_CFG			30
>> +#define SLAVE_BLSP_1			31
>> +#define SLAVE_BLSP_2			32
>> +#define SLAVE_TLMM_NORTH		33
>> +#define SLAVE_PCIE			34
>> +#define SLAVE_ETHERNET			35
>> +#define SLAVE_TLMM_EAST			36
>> +#define SLAVE_TCU			37
>> +#define SLAVE_PMIC_ARB			38
>> +#define SLAVE_SDCC_1			39
>> +#define SLAVE_SDCC_2			40
>> +#define SLAVE_TLMM_SOUTH		41
>> +#define SLAVE_USB_HS			42
>> +#define SLAVE_USB3			43
>> +#define SLAVE_CRYPTO_0_CFG		44
>> +#define SLAVE_PCNOC_SNOC		45
>> +
>> +#define MASTER_QDSS_BAM			0
>> +#define MASTER_BIMC_SNOC		1
>> +#define MASTER_PCNOC_SNOC		2
>> +#define MASTER_QDSS_ETR			3
>> +#define MASTER_EMAC			4
>> +#define MASTER_PCIE			5
>> +#define MASTER_USB3			6
>> +#define QDSS_INT			7
>> +#define SNOC_INT_0			8
>> +#define SNOC_INT_1			9
>> +#define SNOC_INT_2			10
>> +#define SLAVE_KPSS_AHB			11
>> +#define SLAVE_WCSS			12
>> +#define SLAVE_SNOC_BIMC_1		13
>> +#define SLAVE_IMEM			14
>> +#define SLAVE_SNOC_PCNOC		15
>> +#define SLAVE_QDSS_STM			16
>> +#define SLAVE_CATS_0			17
>> +#define SLAVE_CATS_1			18
>> +#define SLAVE_LPASS			19
>> +
>> +#endif