linux-kernel - Re: [PATCH 06/44] staging: spmi: hisi-spmi-controller: use le32 macros where needed

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:   Wed, 12 Aug 2020 16:02:09 -0300
From:   Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To:     Joe Perches <joe@...ches.com>
Cc:     linuxarm@...wei.com, mauro.chehab@...wei.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org
Subject: Re: [PATCH 06/44] staging: spmi: hisi-spmi-controller: use le32
 macros where needed

Em Wed, 12 Aug 2020 09:21:54 -0700
Joe Perches <joe@...ches.com> escreveu:

> On Wed, 2020-08-12 at 17:56 +0200, Mauro Carvalho Chehab wrote:
> > Instead of manually using bswap_32(), just use the
> > le32 macros.  
> 
> Are you certain this code will now work on any endian cpu?
> 
> Maybe just use __swab32 instead

Well, I didn't test, because this driver is for an specific
hardware (arm64). Yet, what happens in practice is that just
one byte is written by the PMIC drivers. If the order is not
LE, the byte written at the buffer will always be zero.

> 
> > diff --git a/drivers/staging/hikey9xx/hisi-spmi-controller.c b/drivers/staging/hikey9xx/hisi-spmi-controller.c  
> []
> > @@ -43,11 +42,6 @@
> >  #define SPMI_APB_SPMI_CMD_TYPE_OFFSET			24
> >  #define SPMI_APB_SPMI_CMD_LENGTH_OFFSET			20
> >  
> > -#define bswap_32(X)   \
> > -    ((((u32)(X) & 0xff000000) >> 24) | \
> > -     (((u32)(X) & 0x00ff0000) >> 8) | \
> > -     (((u32)(X) & 0x0000ff00) << 8) | \
> > -     (((u32)(X) & 0x000000ff) << 24))
> >  #define SPMI_APB_SPMI_CMD_SLAVEID_OFFSET		16
> >  #define SPMI_APB_SPMI_CMD_ADDR_OFFSET			0
> >  
> > @@ -179,14 +173,15 @@ static int spmi_read_cmd(struct spmi_controller *ctrl,
> >  
> >  	writel(cmd, spmi_controller->base + chnl_ofst + SPMI_APB_SPMI_CMD_BASE_ADDR);
> >  
> > -	rc = spmi_controller_wait_for_done(spmi_controller, spmi_controller->base, sid, addr);
> > +	rc = spmi_controller_wait_for_done(spmi_controller,
> > +					   spmi_controller->base, sid, addr);
> >  	if (rc)
> >  		goto done;
> >  
> >  	i = 0;
> >  	do {
> >  		data = readl(spmi_controller->base + chnl_ofst + SPMI_SLAVE_OFFSET * sid + SPMI_APB_SPMI_RDATA0_BASE_ADDR + i * SPMI_PER_DATAREG_BYTE);
> > -		data = bswap_32(data);
> > +		data = be32_to_cpu((__be32)data);
> >  		if ((bc - i * SPMI_PER_DATAREG_BYTE) >> 2) {
> >  			memcpy(buf, &data, sizeof(data));
> >  			buf += sizeof(data);
> > @@ -210,8 +205,7 @@ static int spmi_write_cmd(struct spmi_controller *ctrl,
> >  {
> >  	struct spmi_controller_dev *spmi_controller = dev_get_drvdata(&ctrl->dev);
> >  	unsigned long flags;
> > -	u32 cmd;
> > -	u32 data = 0;
> > +	u32 cmd, data;
> >  	int rc;
> >  	u32 chnl_ofst = SPMI_CHANNEL_OFFSET * spmi_controller->channel;
> >  	u8 op_code, i;
> > @@ -246,7 +240,7 @@ static int spmi_write_cmd(struct spmi_controller *ctrl,
> >  
> >  	i = 0;
> >  	do {
> > -		memset(&data, 0, sizeof(data));
> > +		data = 0;
> >  		if ((bc - i * SPMI_PER_DATAREG_BYTE) >> 2) {
> >  			memcpy(&data, buf, sizeof(data));
> >  			buf += sizeof(data);
> > @@ -255,8 +249,8 @@ static int spmi_write_cmd(struct spmi_controller *ctrl,
> >  			buf += (bc % SPMI_PER_DATAREG_BYTE);
> >  		}
> >  
> > -		data = bswap_32(data);
> > -		writel(data, spmi_controller->base + chnl_ofst + SPMI_APB_SPMI_WDATA0_BASE_ADDR + SPMI_PER_DATAREG_BYTE * i);
> > +		writel((u32)cpu_to_be32(data),
> > +		       spmi_controller->base + chnl_ofst + SPMI_APB_SPMI_WDATA0_BASE_ADDR + SPMI_PER_DATAREG_BYTE * i);
> >  		i++;
> >  	} while (bc > i * SPMI_PER_DATAREG_BYTE);
> >    
>