linux-kernel - RE: [PATCH 2/2 v2] SPI: spi-pxa2xx: SPI support for Intel Quark X1000

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]

Message-ID: <4656BEB6164FC34F8171C6538F1A595B2E997C63@SHSMSX101.ccr.corp.intel.com>
Date:	Wed, 8 Oct 2014 01:32:09 +0000
From:	"Chen, Alvin" <alvin.chen@...el.com>
To:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC:	Eric Miao <eric.y.miao@...il.com>,
	Russell King <linux@....linux.org.uk>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Mark Brown <broonie@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Westerberg, Mika" <mika.westerberg@...el.com>,
	"Kweh, Hock Leong" <hock.leong.kweh@...el.com>,
	"Ong, Boon Leong" <boon.leong.ong@...el.com>,
	"Tan, Raymond" <raymond.tan@...el.com>
Subject: RE: [PATCH 2/2 v2] SPI: spi-pxa2xx: SPI support for Intel Quark
 X1000

> > The SPI memory mapped I/O registers supported by Quark are different
> > from the current implementation, and Quark only supports the registers
> > of 'SSCR0', 'SSCR1', 'SSSR', 'SSDR', and 'DDS_RATE'. This patch is to
> > enable the SPI for Intel Quark X1000.
> >
> > This piece of work is derived from Dan O'Donovan's initial work for
> > Intel Quark
> > X1000 SPI enabling.
> 
> Minor comments are below.
> 
> After addressing them
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> 
OK.
> > +	case QUARK_X1000_SSP:
> > +		return RX_THRESH_QUARK_X1000_DFLT;
> >  	default:
> >  		return RX_THRESH_DFLT;
> >  	}
> > +
> 
> Redundant empty line.
> 
OK. I will remove it.

> >  static void pxa2xx_spi_clear_rx_thre(const struct driver_data *drv_data,
> > -				     u32 *sccr1_reg)
> > +					  u32 *sccr1_reg)
> 
> Unnecessary change.
> 
Improve it in the first patch.
> >  static void pxa2xx_spi_set_rx_thre(const struct driver_data *drv_data,
> > -				   u32 *sccr1_reg, u32 threshold)
> > +					u32 *sccr1_reg, u32 threshold)
> 
> Ditto.
> 
> Or you may do that in the first patch.
> 
Improve it in the first patch.
> >
> >  			pxa2xx_spi_set_rx_thre(drv_data, &sccr1_reg,
> > -					       rx_thre);
> > +						    rx_thre);
> 
> Ditto.
> 
Improve it in the first patch.
> > +	if (is_quark_x1000_ssp(drv_data) &&
> > +	    (read_DDS_RATE(reg) != chip->dds_rate))
> 
> Could it be one line?
> 
No, it is beyond 80 characters if it is in one line.