Re: [PATCH v7 1/5] dt-bindings: interconnect: Add Qualcomm IPQ9574 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/10/24 13:15, Krzysztof Kozlowski wrote:
On 10/04/2024 12:02, Varadarajan Narayanan wrote:
Okay, so what happens if icc-clk way of generating them changes a bit?
It can change, why not, driver implementation is not an ABI.


	2. These auto-generated id-numbers have to be correctly
	   tied to the DT nodes. Else, the relevant clocks may
	   not get enabled.

Sorry, I don't get, how auto generated ID number is tied to DT node.
What DT node?

I meant the following usage for the 'interconnects' entry of the
consumer peripheral's node.

	interconnects = <&gcc MASTER_ANOC_PCIE0 &gcc SLAVE_ANOC_PCIE0>,
			      ^^^^^^^^^^^^^^^^^      ^^^^^^^^^^^^^^^^
			<&gcc MASTER_SNOC_PCIE0 &gcc SLAVE_SNOC_PCIE0>;
			      ^^^^^^^^^^^^^^^^^      ^^^^^^^^^^^^^^^^

Since ICC-CLK creates two ids per clock entry (one MASTER_xxx and
one SLAVE_xxx), using those MASTER/SLAVE_xxx macros as indices in
the below array would create holes.

	static int icc_ipq9574_hws[] = {
		[MASTER_ANOC_PCIE0] = GCC_ANOC_PCIE0_1LANE_M_CLK,
		[MASTER_SNOC_PCIE0] = GCC_SNOC_PCIE0_1LANE_S_CLK,
		[MASTER_ANOC_PCIE1] = GCC_ANOC_PCIE1_1LANE_M_CLK,
		[MASTER_SNOC_PCIE1] = GCC_SNOC_PCIE1_1LANE_S_CLK,
		. . .
	};

Other Qualcomm drivers don't have this issue and they can
directly use the MASTER/SLAVE_xxx macros.

I understand, thanks, yet your last patch keeps adding fake IDs, means
IDs which are not part of ABI.


As the MASTER_xxx macros cannot be used, have to define a new set
of macros that can be used for indices in the above array. This
is the reason for the ICC_BINDING_NAME macros.

Then maybe fix the driver, instead of adding something which is not an
ABI to bindings and completely skipping the actual ABI.

Will remove the ICC_xxx defines from the header. And in the
driver will change the declaration as follows. Will that be
acceptable?

	static int icc_ipq9574_hws[] = {
		[MASTER_ANOC_PCIE0 / 2] = GCC_ANOC_PCIE0_1LANE_M_CLK,

What is the binding in such case? What exactly do you bind between
driver and DTS?

I think what Krzysztof is trying to say here is "the icc-clk API is tragic"
and the best solution would be to make it such that the interconnect indices
are set explicitly, instead of (master, slave), (master, slave) etc.

Does that sound good, Krzysztof?

Konrad




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux