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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250113192002.39dda2c2@kernel.org>
Date: Mon, 13 Jan 2025 19:20:02 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Divya Koppera <divya.koppera@...rochip.com>
Cc: <andrew@...n.ch>, <arun.ramadoss@...rochip.com>,
 <UNGLinuxDriver@...rochip.com>, <hkallweit1@...il.com>,
 <linux@...linux.org.uk>, <davem@...emloft.net>, <edumazet@...gle.com>,
 <pabeni@...hat.com>, <netdev@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <richardcochran@...il.com>,
 <vadim.fedorenko@...ux.dev>
Subject: Re: [PATCH net-next v2 3/3] net: phy: microchip_rds_ptp : Add
 PEROUT feature library for RDS PTP supported phys

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