Re: [Skiboot] [PATCH V4 3/3] skiboot: Update IMC PMU node names for power10

On Mon, Mar 06, 2023 at 09:09:13AM +0530, Athira Rajeev wrote:

+	} else if (proc_gen == proc_gen_p10) {
+		int val;
+		char al[8], xl[8], otl[8], phb[8];

Are four different variables needed here? I think you could just reuse 
one temporary variable.

		char name[8];

+		for (i=0; i<8; i++) {
+			val = ((avl_vec & (0x7ULL << (29 + (3 * i)))) >> (29 + (3 * i)));
+			switch (val) {
+			case 0x5: //xlink configured and functional
+
+				snprintf(al, sizeof(al), "alink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, al);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl, sizeof(otl),"otl%1d_0",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl,sizeof(otl),"otl%1d_1",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+
+				break;
+			case 0x6: //alink configured and functional
+
+				snprintf(xl,sizeof(xl),"xlink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, xl);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl,sizeof(otl),"otl%1d_0",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl,sizeof(otl),"otl%1d_1",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+				break;
+
+			case 0x7: //CAPI configured and functional
+				snprintf(al,sizeof(al),"alink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, al);
+				if (target)
+					dt_free(target);
+
+				snprintf(xl,sizeof(xl),"xlink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, xl);
+				if (target)
+					dt_free(target);
+				break;
+			default:
+				snprintf(xl,sizeof(xl),"xlink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, xl);
+				if (target)
+					dt_free(target);
+
+				snprintf(al,sizeof(al),"alink%1d",(7-i));
+				target = dt_find_by_name_substr(dev, al);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl,sizeof(otl),"otl%1d_0",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+
+				snprintf(otl,sizeof(otl),"otl%1d_1",(7-i));
+				target = dt_find_by_name_substr(dev, otl);
+				if (target)
+					dt_free(target);
+				break;

As far as I know skiboot follows the kernel coding style. Would you mind 
fixing up the minor style nits checkpatch.pl reports for this patch?

-- 
Reza Arbab