Received-SPF: pass (google.com: domain of linux-kernel+bounces-210646-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Witold Sadowski <wsadowski@marvell.com>
To: Mark Brown <broonie@kernel.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "robh@kernel.org"
	<robh@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
        "conor+dt@kernel.org"
	<conor+dt@kernel.org>,
        "pthombar@cadence.com" <pthombar@cadence.com>
Subject: RE: [EXTERNAL] Re: [PATCH v8 4/4] spi: cadence: Add MRVL overlay xfer
 operation support
Thread-Topic: [EXTERNAL] Re: [PATCH v8 4/4] spi: cadence: Add MRVL overlay
 xfer operation support
Thread-Index: AQHauO4LZaM3qdOTaUCf+ifD3OpGhLHBMgQAgAHwyyA=
Date: Tue, 11 Jun 2024 22:02:33 +0000
Message-ID: 
 <CO6PR18MB40985A453930C40DCB5C5845B0C72@CO6PR18MB4098.namprd18.prod.outlook.com>
References: <20240607151831.3858304-1-wsadowski@marvell.com>
 <20240607151831.3858304-5-wsadowski@marvell.com>
 <ZmcnhGH2fcmrXn1G@finisterre.sirena.org.uk>
In-Reply-To: <ZmcnhGH2fcmrXn1G@finisterre.sirena.org.uk>
Accept-Language: en-US, pl-PL
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?f+gokFDPRYgJIUGNYsvYMi3zup83jj0iE9Ot4i64Gk106p2CeomPlSokvMkZ?=
 =?us-ascii?Q?hNasG052nF6bGlZydan8ubQh4x/I4EPWOADeZEda6TEiGWGYeVJd8zxvodTz?=
 =?us-ascii?Q?77WFQuwxEGV5EHjoNtmhRmaqUF37NDQ344K9DlxGG48Pxk4cEYW2I9hGL081?=
 =?us-ascii?Q?oeyoYrQzBh5i5aq0PzPlo3zdizpz0AxDbdYeraFkdymSTDpG5iTLA5byWYV8?=
 =?us-ascii?Q?OkaLksf9rEUuLkFl2MDQQn/Eg6Sa7XbgtaB46takHZez0yQLK7c3Iuxfcv8F?=
 =?us-ascii?Q?GqfQY7LNa2NWNh/qM2cUYDU3jGh7eweNf/KjeAvrJ/4+j23iUp2UT0VBjrMB?=
 =?us-ascii?Q?ZA8mjQnM6bB4JrTtuSapSqP3C6fVHsppX9bhrh3S2e0dzFON3SeP6vq7/PU0?=
 =?us-ascii?Q?Z2Pwx3rUInsqeTBWAw5gpzPUw3/a7yfhsK/EHbBeZu+T5sC377zsC0ltZtov?=
 =?us-ascii?Q?cv9ncpDAUqQ64ku1U7hde9azERh2zW6Cj3SX/qMFE4fndVCUEPwcHpuLOI6S?=
 =?us-ascii?Q?vJd1jUrg5gqDv3z3ClYXi1b97Ny0XMUpm3ovHejFmp5EMLv7wcAp9/EArTsQ?=
 =?us-ascii?Q?lJj3o0rXi5YW8WR57UObuVwfH1IWLXnE0CfCXMZPiS4Z83N5XNzoNa3EZS/i?=
 =?us-ascii?Q?hiY6hL8kWVg+g5HuK3SqnJtadwpil8ioCbFoUEzhSH/S8YrjwMQLGjCrjRRl?=
 =?us-ascii?Q?X9D+eK9Q19vwzuTjgRUOXnTz8D5wfKZoqBO762M/YpLw2Mr3cEFXce8tnacT?=
 =?us-ascii?Q?qz2OpNIFCcFNdiuwIoNTIUEewlGo5xcyav835obIPsW90hFyeod+t7QXs0r8?=
 =?us-ascii?Q?l+ZvraD/I8o4WhgyH9bSgM85K5dG+BwOhByx9bboUlxyVSHNigXanRI0JkVS?=
 =?us-ascii?Q?we8+G/JgaXZ8jbskHZRo+ssKDJPYays/p8zQqf20VmCaxnKfQ1SbpzQQUfvX?=
 =?us-ascii?Q?+ZXDI4rVoORX4PHHxdPsdoW9o6ez64UH/Fxb1f/iyyJ08wpD2DhzS7Hwweu1?=
 =?us-ascii?Q?ezJ8jJ6ta/NIxg5ZtaJ/dQVBbvlPlad3Q3xUzEZTPcTgck5Dt0U5XxEaF/9/?=
 =?us-ascii?Q?3aB/krK9kLF4UrhZpIQgbQYBYUDgMtnfLttta/zqR2DuZUaULIRoU1yC0o8/?=
 =?us-ascii?Q?piSk/2J8ltvMj1bjul0O0e3D0zXw+VV8yE+/CoiBsGYpMTe/QQynIa78XnT6?=
 =?us-ascii?Q?oI43AlZ36RcWfdlYpjU5P3EKQSNalfrVovebYzBed6HUH4DnXI0Bh6q1Vd0e?=
 =?us-ascii?Q?X/cithyACoWIoHptZR09OirSzO1ErL18BW2ePi4hcA3KQtO6Hsn66qVXVE62?=
 =?us-ascii?Q?WwoYob9sXeNteKnGeZvhCZO5XKrc2+UvTREa9+4413tHNr9hKBv/6vhd+qwx?=
 =?us-ascii?Q?ghEiQXo1dwdpYmuQ8H/4MIuttm0b6WqcME28FHXteUmjLdCJCnJ4PtR9Q0Wc?=
 =?us-ascii?Q?HE4zdwN1eKTOxUfntk5RDQlr1lccFab7KOPshq9F2zW/nubp5JSrRg4meCv3?=
 =?us-ascii?Q?Hdrb6aegofHfp6srv2+B7oePM9QimpLKLWiHThOf8gaJYVAkx4qKbT3s33oB?=
 =?us-ascii?Q?Um20nH98hUHVLD3fx5qGE+li8a+GqgWW73tzLxYH?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4098.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f8fe99c7-ab5f-4dea-ccd5-08dc8a62328e
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jun 2024 22:02:33.9123
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: yT4dLayYJ3xvwiP1StfAzM9GX4WokSvWs+Fkasri1sqsiBWQPknQ0miqQvo0/L0utL0S50ApVRgOLM0SFzveqA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR18MB4149
X-Proofpoint-GUID: GQnPyGOK_ms24oecs_eNDTivaEeUBI45
X-Proofpoint-ORIG-GUID: GQnPyGOK_ms24oecs_eNDTivaEeUBI45
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-11_11,2024-06-11_01,2024-05-17_01

Hi

>=20
> > +static int cdns_xspi_prepare_generic(int cs, const void *dout, int
> > +len, int glue, u32 *cmd_regs) {
> > +	u8 *data =3D (u8 *)dout;
> > +	int i;
> > +	int data_counter =3D 0;
> > +
> > +	memset(cmd_regs, 0x00, 6*4);
>=20
> The magic numbers here aren't great...

That will be fixed.

>=20
> > +static unsigned char reverse_bits(unsigned char num) {
> > +	unsigned int count =3D sizeof(num) * 8 - 1;
> > +	unsigned int reverse_num =3D num;
> > +
> > +	num >>=3D 1;
> > +	while (num) {
> > +		reverse_num <<=3D 1;
> > +		reverse_num |=3D num & 1;
> > +		num >>=3D 1;
> > +		count--;
> > +	}
> > +	reverse_num <<=3D count;
> > +	return reverse_num;
> > +}
>=20
> 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 make=
s
> sense to add this as one.
>=20
> > +	/* Enable xfer state machine */
> > +	if (!cdns_xspi->xfer_in_progress) {
> > +		u32 xfer_control =3D readl(cdns_xspi->xferbase +
> > +MRVL_XFER_FUNC_CTRL);
> > +
> > +		cdns_xspi->current_xfer_qword =3D 0;
> > +		cdns_xspi->xfer_in_progress =3D true;
> > +		xfer_control |=3D (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 &=3D ~(MRVL_XFER_FUNC_ENABLE |
> MRVL_XFER_CLK_DRIVE_POL);
> > +		writel(xfer_control, cdns_xspi->xferbase +
> MRVL_XFER_FUNC_CTRL);
> > +	}
>=20
> Could this just be a prepare_transfer_hardware() and we could just use
> transfer_one()?

I have to run some experiments, but it should be possible.

>=20
> > +	list_for_each_entry(t, &m->transfers, transfer_list) {
> > +		u8 *txd =3D (u8 *) t->tx_buf;
> > +		u8 *rxd =3D (u8 *) t->rx_buf;
> > +		u8 data[10];
> > +		u32 cmd_regs[6];
> > +
> > +		if (!txd)
> > +			txd =3D data;
> > +
> > +		cdns_xspi->in_buffer =3D txd + 1;
> > +		cdns_xspi->out_buffer =3D txd + 1;
>=20
> Oh?

You are asking about that 1 byte offset? It is caused by way that
SDMA is handled in that specific case - all data except of first
byte is transferred via SDMA, the first byte is send in command, and
SDMA is not involved in that.

Regards
Witek