Re: [PATCH v5 3/3] regulator: tps6594-regulator: Add driver for TI TPS6594 regulators

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

 





+	enum {
+		MULTI_BUCK12,
+		MULTI_BUCK123,
+		MULTI_BUCK1234,
+		MULTI_BUCK12_34,

+		MULTI_FIRST = MULTI_BUCK12,
+		MULTI_LAST = MULTI_BUCK12_34,
+		MULTI_NUM = MULTI_LAST - MULTI_FIRST + 1

		MULT_NUM

will suffice instead all this.

+	};

But why enum at all? See below.
Just for the switch case readability.
I have to iterate across the multiphases array for look up name into device tree and evaluate in that order.

This can be reduced to:
	enum {
		MULTI_BUCK12,
		MULTI_BUCK123,
		MULTI_BUCK1234,
		MULTI_BUCK12_34,
		MULTI_NUM = MULTI_BUCK12_34 - MULTI_BUCK12 + 1
	};


...

+	/*
+	 * Switch case defines different possible multi phase config
+	 * This is based on dts buck node name.
+	 * Buck node name must be chosen accordingly.
+	 * Default case is no Multiphase buck.
+	 * In case of Multiphase configuration, value should be defined for
+	 * buck_configured to avoid creating bucks for every buck in multiphase
+	 */
+	for (multi = MULTI_FIRST; multi < MULTI_NUM; multi++) {
+		np = of_find_node_by_name(tps->dev->of_node, multiphases[multi]);
+		npname = of_node_full_name(np);
+		np_pmic_parent = of_get_parent(of_get_parent(np));
+		if (of_node_cmp(of_node_full_name(np_pmic_parent), tps->dev->of_node->full_name))

Why not of_node_full_name() in the second case?
Sure.


+			continue;
+		delta = strcmp(npname, multiphases[multi]);
+		if (!delta) {
+			switch (multi) {
+			case MULTI_BUCK12:

This all looks like match_string() reinvention.
I can go with match_string but this is not significantly changing the game:

index = match_string(multiphases, ARRAY_SIZE(multiphases), npname);
if (index >= 0) {
	switch (index) {

No question on all your other feedback. Just wondering if I missed something with match_string use. Looks like a good idea indeed but this is not drastically changing the code as you seem to expect... Let me know if you think I'm doing it in a wrong way.

Regards,
Jerome.



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux