Messages in this thread | | | Subject | Re: [PATCHv2] reset: socfpga: add an early reset driver for SoCFPGA | From | Philipp Zabel <> | Date | Wed, 17 Oct 2018 17:52:32 +0200 |
| |
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
| |