linux-kernel - RE: [PATCH 3/6] spi: Add driver for the RZ/V2H(P) RSPI IP

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <TYCPR01MB1209371D193D48360CD5D09A7C242A@TYCPR01MB12093.jpnprd01.prod.outlook.com>
Date: Fri, 4 Jul 2025 14:17:23 +0000
From: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
To: Mark Brown <broonie@...nel.org>
CC: Philipp Zabel <p.zabel@...gutronix.de>, Geert Uytterhoeven
	<geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, Biju
 Das <biju.das.jz@...renesas.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH 3/6] spi: Add driver for the RZ/V2H(P) RSPI IP

Hi Mark,

Thanks for your feedback.

> From: Mark Brown <broonie@...nel.org>
> Sent: 25 June 2025 20:09
> To: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> Cc: Philipp Zabel <p.zabel@...gutronix.de>; Geert Uytterhoeven <geert+renesas@...der.be>; Magnus Damm
> <magnus.damm@...il.com>; linux-kernel@...r.kernel.org; linux-spi@...r.kernel.org; linux-renesas-
> soc@...r.kernel.org; Biju Das <biju.das.jz@...renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@...renesas.com>
> Subject: Re: [PATCH 3/6] spi: Add driver for the RZ/V2H(P) RSPI IP
> 
> On Tue, Jun 24, 2025 at 08:23:01PM +0100, Fabrizio Castro wrote:
> 
> > +static int rzv2h_rspi_unprepare_message(struct spi_controller *ctlr,
> > +					struct spi_message *message)
> > +{
> > +	struct rzv2h_rspi_priv *rspi = spi_controller_get_devdata(ctlr);
> > +
> > +	rzv2h_rspi_spe_disable(rspi);
> > +	rzv2h_rspi_clear_fifos(rspi);
> 
> A bit interesting that we need to clear the FIFOs, but it's just one
> register write so probably not worth worrying about.

I can surely take out rzv2h_rspi_clear_fifos() from
rzv2h_rspi_unprepare_message(), as rzv2h_rspi_prepare_message()
makes sure they are clear before we start transferring.

> 
> > +	ret = devm_clk_bulk_get(dev, RSPI_CLK_NUM, rspi->clks);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "cannot get clocks\n");
> 
> > +	ret = devm_request_irq(dev, irq_rx, rzv2h_rx_irq_handler, 0,
> > +			       dev_name(dev), rspi);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "cannot request `rx` IRQ\n");
> > +
> > +	ret = clk_bulk_prepare_enable(RSPI_CLK_NUM, rspi->clks);
> 
> Are you sure that the interrupt handler is safe with the IP in reset and
> clocks disabled...
> 
> > +	init_waitqueue_head(&rspi->wait);
> 
> ...and the wakequeue head it wakes up not yet initialised?  This is also
> a concern during unregistration where devm things will be unwound after
> the remove() function has run.


Good points, I'll move devm_request_irq() to after the clocks are ON,
the resets are deasserted, and the waitqueue has been initialized.

Thanks,
Fab