linux-kernel - RE: [EXTERNAL] Re: [PATCH v8 4/4] spi: cadence: Add MRVL overlay xfer operation support

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: 
 <CO6PR18MB40984F00856BC88C801ADA88B0CE2@CO6PR18MB4098.namprd18.prod.outlook.com>
Date: Tue, 18 Jun 2024 12:59:43 +0000
From: Witold Sadowski <wsadowski@...vell.com>
To: Mark Brown <broonie@...nel.org>
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 4/4] spi: cadence: Add MRVL overlay xfer
 operation support

Hi

> 
> > +static unsigned char reverse_bits(unsigned char num) {
> > +	unsigned int count = sizeof(num) * 8 - 1;
> > +	unsigned int reverse_num = num;
> > +
> > +	num >>= 1;
> > +	while (num) {
> > +		reverse_num <<= 1;
> > +		reverse_num |= num & 1;
> > +		num >>= 1;
> > +		count--;
> > +	}
> > +	reverse_num <<= count;
> > +	return reverse_num;
> > +}
> 
> I can't help but think there ought to be a helper for this though I can't
> think what it is off the top of my head.  If there isn't it probably makes
> sense to add this as one.

It seems bitrev8 is doing what I want.

> 
> > +	/* Enable xfer state machine */
> > +	if (!cdns_xspi->xfer_in_progress) {
> > +		u32 xfer_control = readl(cdns_xspi->xferbase +
> > +MRVL_XFER_FUNC_CTRL);
> > +
> > +		cdns_xspi->current_xfer_qword = 0;
> > +		cdns_xspi->xfer_in_progress = true;
> > +		xfer_control |= (MRVL_XFER_RECEIVE_ENABLE |
> > +				 MRVL_XFER_CLK_CAPTURE_POL |
> > +				 MRVL_XFER_FUNC_START |
> > +				 MRVL_XFER_SOFT_RESET |
> > +				 FIELD_PREP(MRVL_XFER_CS_N_HOLD, (1 << cs)));
> > +		xfer_control &= ~(MRVL_XFER_FUNC_ENABLE |
> MRVL_XFER_CLK_DRIVE_POL);
> > +		writel(xfer_control, cdns_xspi->xferbase +
> MRVL_XFER_FUNC_CTRL);
> > +	}
> 
> Could this just be a prepare_transfer_hardware() and we could just use
> transfer_one()?

I have run some experiments, and it won't be possible. HW expects reset after
completing whole transfer. With prepare_transfer_hardware() and unprepared_transfer_hardware(),
that reset won't be guaranteed.
Also, there will be a problem when CS line will be changed(without overlay reinit)

Regards
Witek