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

[Paolo Abeni]

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