linux-kernel - Re: [PATCHv2] reset: socfpga: add an early reset driver for SoCFPGA

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: <1539791552.4729.13.camel@pengutronix.de>
Date:   Wed, 17 Oct 2018 17:52:32 +0200
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Dinh Nguyen <dinguyen@...nel.org>
Cc:     linux-kernel@...r.kernel.org, marex@...x.de
Subject: Re: [PATCHv2] reset: socfpga: add an early reset driver for SoCFPGA

Hi Dinh,

On Wed, 2018-10-17 at 10:02 -0500, Dinh Nguyen wrote:
[...]
> > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > > index 13d28fdbdbb5..f10de5ce4753 100644
> > > --- a/drivers/reset/Kconfig
> > > +++ b/drivers/reset/Kconfig
> > > @@ -100,7 +100,7 @@ config RESET_QCOM_AOSS
> > >  
> > >  config RESET_SIMPLE
> > >  	bool "Simple Reset Controller Driver" if COMPILE_TEST
> > > -	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED
> > > +	default ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED
> > >  	help
> > >  	  This enables a simple reset controller driver for reset lines that
> > >  	  that can be asserted and deasserted by toggling bits in a contiguous,
> > > @@ -119,6 +119,13 @@ config RESET_STM32MP157
> > >  	help
> > >  	  This enables the RCC reset controller driver for STM32 MPUs.
> > >  
> > > +config RESET_SOCFPGA
> > > +	bool "SoCFPGA Reset Driver" if COMPILE_TEST && !ARCH_SOCFPGA
> > > +	default ARCH_SOCFPGA && !ARCH_STRATIX10
> > > +	select RESET_SIMPLE
[...]
> > > +static const struct of_device_id socfpga_early_reset_dt_ids[] __initconst = {
> > > +	{ .compatible = "altr,rst-mgr", },
> > 
> > That doesn't seem right. If you don't remove the altr,rst-mgr compatible
> > from reset-simple.c anymore, we suddenly have two device drivers for the
> > same compatible.
> 
> You're right, and that's why I am not building reset-simple for
> ARCH_SOCFPGA anymore. I am only building reset-simple for ARCH_STRATIX10.

RESET_SOCFPGA still selects RESET_SIMPLE, so reset-simple should be
built in both cases.

> > (Also I liked removing altr,modrst-offset from reset-simple.c)
> 
> I can remove it since it's just 0x20 of ARCH_STRATIX10.

As long as there is another driver handling that compatible,
altr,rst-mgr support should be removed from the platform driver in
reset-simple.c altogether.

> > Would there be any issue with calling socfpga_reset_init() on Stratix10
> > as well?
> 
> I don't see any place in the arm64 common code where I can do this.

This is one of the reasons why there should always be a SoC specific
compatible as well as the generic one. If the device trees contained
something like

	compatible = "altr,socfpga-a10-rst-mgr", "altr,rst-mgr";

and

	compatible = "altr,socfpga-s10-rst-mgr", "altr,rst-mgr";

we could now easily match the specific compatibles for the different
cases (s10 in reset-simple, a10 in reset-socfpga).

I suppose one way to handle this with the shared compatible would be to
add a platform driver for s10 to reset-socfpga.c, but register it only
if socfpga_reset_init() was not called earlier.

regards
Philipp