netdev - RE: [PATCH mfd 1/1] mfd/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] [day] [month] [year] [list]

Message-ID: <OS3PR01MB65937690235A4DF614329A96BA879@OS3PR01MB6593.jpnprd01.prod.outlook.com>
Date:   Thu, 23 Mar 2023 14:18:16 +0000
From:   Min Li <min.li.xe@...esas.com>
To:     Jakub Kicinski <kuba@...nel.org>, Min Li <lnimi@...mail.com>
CC:     "richardcochran@...il.com" <richardcochran@...il.com>,
        "lee@...nel.org" <lee@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH mfd 1/1] mfd/ptp: clockmatrix: support 32-bit address
 space

> On Tue, 21 Mar 2023 15:10:06 -0400 Min Li wrote:
> > -		buf[0] = (u8)(page & 0xff);
> > -		buf[1] = (u8)((page >> 8) & 0xff);
> > +		buf[0] = (u8)(page & 0xFF);
> > +		buf[1] = (u8)((page >> 8) & 0xFF);
> 
> why did you decide to change from 0xff to 0xFF as part of this big change?
> It's unnecessary churn.

I will revert it back to 0xff

> > -		err = idtcm_write(idtcm, 0, HW_Q8_CTRL_SPARE,
> > +		err = idtcm_write(idtcm, HW_Q8_CTRL_SPARE, 0,
> >  				  &temp, sizeof(temp));
> >  		if (err)
> >  			return err;
> >
> >  		temp |= Q9_TO_Q8_SYNC_TRIG;
> >
> > -		err = idtcm_write(idtcm, 0, HW_Q8_CTRL_SPARE,
> > +		err = idtcm_write(idtcm, HW_Q8_CTRL_SPARE, 0,
> >  				  &temp, sizeof(temp));
> >  		if (err)
> >  			return err;
> 
> Why are you flipping all these arguments?
> Isn't HW_Q8_CTRL_SPARE regaddr?

static inline int idtcm_write(struct idtcm *idtcm,
-			      u16 module,
+			      u32 module,
 			      u16 regaddr,
 			      u8 *buf,
 			      u16 count)

HW_Q8_CTRL_SPARE is the module and is a u32. regaddr is the offset within the module. 
They used to be both u16 but after the change, module becomes u32.

> Could you to split your patches into multiple steps to make them easier to
> reivew?

I will split the change into ptp and mfd respectively.