netdev - Re: [PATCH net-next v6 1/6] ptp: clockmatrix: support 32-bit address space

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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 05 Dec 2023 10:06:46 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Min Li <lnimi@...mail.com>, richardcochran@...il.com, lee@...nel.org
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Min Li
	 <min.li.xe@...esas.com>
Subject: Re: [PATCH net-next v6 1/6] ptp: clockmatrix: support 32-bit
 address space

Hi,

On Thu, 2023-11-30 at 13:46 -0500, Min Li wrote:
> diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
> index f6f9d4adce04..f8556627befa 100644
> --- a/drivers/ptp/ptp_clockmatrix.c
> +++ b/drivers/ptp/ptp_clockmatrix.c
> @@ -41,8 +41,8 @@ module_param(firmware, charp, 0);
>  static int _idtcm_adjfine(struct idtcm_channel *channel, long scaled_ppm);
>  
>  static inline int idtcm_read(struct idtcm *idtcm,
> -			     u16 module,
> -			     u16 regaddr,
> +			     u32 module,
> +			     u32 regaddr,
>  			     u8 *buf,
>  			     u16 count)
>  {

[...]
> @@ -553,11 +554,11 @@ static int _sync_pll_output(struct idtcm *idtcm,
>  	val = SYNCTRL1_MASTER_SYNC_RST;
>  
>  	/* Place master sync in reset */
> -	err = idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
> +	err = idtcm_write(idtcm, sync_ctrl1, 0, &val, sizeof(val));
>  	if (err)
>  		return err;

I'm sorry, but it looks like my previous feedback was not clear enough:
now that the 'regaddr' argument in idtcm_write() has 'u32' type, you
don't need anymore to swap the 'module' and 'regaddr' arguments in the
call site.

Specifically, you can drop the above chunk, many later similar ones 
and additionally even chunks calling to idtcm_write(), for the same
reason.

That would make the patch smaller, and IMHO more clear: it would be 
strange that something that was known as a register address suddenly
become a 'module'.

Thanks!

Paolo