linux-kernel - Re: [EXTERNAL] Re: [PATCH v8 2/4] spi: cadence: Add Marvell xSPI IP overlay changes

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: <Zml5DD5s7OZopVkU@finisterre.sirena.org.uk>
Date: Wed, 12 Jun 2024 11:31:40 +0100
From: Mark Brown <broonie@...nel.org>
To: Witold Sadowski <wsadowski@...vell.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"robh@...nel.org" <robh@...nel.org>,
	"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	"pthombar@...ence.com" <pthombar@...ence.com>
Subject: Re: [EXTERNAL] Re: [PATCH v8 2/4] spi: cadence: Add Marvell xSPI IP
 overlay changes

On Tue, Jun 11, 2024 at 09:51:58PM +0000, Witold Sadowski wrote:
> > On Fri, Jun 07, 2024 at 08:18:29AM -0700, Witold Sadowski wrote:

> > > features included are:
> > >     - Clock configuration
> > >     - PHY configuration
> > >     - Interrupt configuration (enabling)

> > This feels like it could usefully be split up so these three bits are
> > separate, and there appear to be other changes buried in here as well.
> > I can't tell what changes either the PHY or interrupt configuration might
> > be referencing.

> That changes are in single commit as, using not all of them will result in
> total xSPI failure. Configuring PHY makes no sense if clock is not enabled.
> But I can try to split that into 3 separate commits.

They won't actually do anything until we detect the Marvell IP.

> > > +static void mrvl_ioreadq(void __iomem  *addr, void *buf, int len) {
> > > +	int i = 0;
> > > +	int rcount = len / 8;
> > > +	int rcount_nf = len % 8;
> > > +	uint64_t tmp;
> > > +	uint64_t *buf64 = (uint64_t *)buf;

> > Any need to cast away from void * indicates a problem.

> I will check that, but code is checking alignment of that pointer.

A cast won't do anything to fix alignment issues.

> > >  	case CDNS_XSPI_SDMA_DIR_READ:
> > > -		ioread8_rep(cdns_xspi->sdmabase,
> > > -			    cdns_xspi->in_buffer, sdma_size);
> > > +		cdns_xspi_sdma_memread(cdns_xspi, sdma_size);
> > >  		break;

> > It's feeling like it might make sense to have an ops structure rather than
> > sprinkling checks for the Marvell overlay everywhere.

> Won't it cause big code duplication? There are some differences, but whole
> Part of SPI stig mode configuration is the same.

No more than having a bunch of functions which are called a single time
with checks in them will?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)