Re: [PATCH net-next v2 3/3] net: phy: microchip_rds_ptp : Add PEROUT feature library for RDS PTP supported phys

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 9 Jan 2025 15:55:33 +0530 Divya Koppera wrote:
> +	switch (ts_on_nsec) {
> +	case 200000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_200MS_;
> +		break;
> +	case 100000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100MS_;
> +		break;
> +	case 50000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_50MS_;
> +		break;
> +	case 10000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_10MS_;
> +		break;
> +	case 5000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_5MS_;
> +		break;
> +	case 1000000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_1MS_;
> +		break;
> +	case 500000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_500US_;
> +		break;
> +	case 100000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100US_;
> +		break;
> +	case 50000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_50US_;
> +		break;
> +	case 10000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_10US_;
> +		break;
> +	case 5000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_5US_;
> +		break;
> +	case 1000:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_1US_;
> +		break;
> +	case 500:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_500NS_;
> +		break;
> +	case 100:
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100NS_;
> +		break;
> +	default:
> +		phydev_warn(phydev, "Using default pulse width of 200ms\n");
> +		*pulse_width = MCHP_RDS_PTP_GEN_CFG_LTC_EVT_200MS_;
> +		break;

+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_200MS_	13
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100MS_	12
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_50MS_	11
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_10MS_	10
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_5MS_	9
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_1MS_	8
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_500US_	7
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100US_	6
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_50US_	5
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_10US_	4
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_5US_	3
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_1US_	2
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_500NS_	1
+#define MCHP_RDS_PTP_GEN_CFG_LTC_EVT_100NS_	0

1) seems a bit weird to me that you go to 200ms whenever user asks for
   a value that couldn't be provided exactly. Why not go to the next
   good value?
2) this is not very well coded up, given that the values you're
   translating to are a just natural numbers you can use a table

static const sup_on_necs[] = {
	100,		/* 100ns */
	500,		/* 500ns */
	1000,		/* 1us */
	5000,		/* 5us */
	...
};

for (i = 0; i < ARRAY_SIZE(sup_on_necs); i++) {
	if (ts_on_nsec <= sup_on_necs[i]) {
		*pulse_width = i;
		break;
	}
}
-- 
pw-bot: cr