linux-kernel - Re: [PATCH v4 01/10] ARM: sun9i: smp: Add sun9i dt parsing function

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180225161901.207b69fe@dell-desktop.home>
Date:   Sun, 25 Feb 2018 16:19:01 +0100
From:   Mylène Josserand <mylene.josserand@...tlin.com>
To:     Maxime Ripard <maxime.ripard@...tlin.com>
Cc:     linux@...linux.org.uk, wens@...e.org, robh+dt@...nel.org,
        mark.rutland@....com, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        clabbe.montjoie@...il.com, thomas.petazzoni@...tlin.com,
        quentin.schulz@...tlin.com
Subject: Re: [PATCH v4 01/10] ARM: sun9i: smp: Add sun9i dt parsing function

Hi,

On Fri, 23 Feb 2018 15:54:23 +0100
Maxime Ripard <maxime.ripard@...tlin.com> wrote:

> Hi,
> 
> On Fri, Feb 23, 2018 at 02:37:33PM +0100, Mylène Josserand wrote:
> > To prepare the support for sun8i-a83t, create a new function
> > that handles all the resources needed on sun9i-a80 (because
> > it will be different from sun8i-a83t).
> > 
> > All the parsing of device tree is moved into this new function:
> > sun9i_dt_parsing. Create also a function to release the resources
> > retrieved during the dt parsing in case there is an error in init
> > function.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@...tlin.com>
> > ---
> >  arch/arm/mach-sunxi/mc_smp.c | 99 ++++++++++++++++++++++++++------------------
> >  1 file changed, 58 insertions(+), 41 deletions(-)
> > 
> > diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> > index 11e46c6efb90..650a2ad4398f 100644
> > --- a/arch/arm/mach-sunxi/mc_smp.c
> > +++ b/arch/arm/mach-sunxi/mc_smp.c
> > @@ -684,35 +684,22 @@ static int __init sunxi_mc_smp_lookback(void)
> >  	return ret;
> >  }
> >  
> > -static int __init sunxi_mc_smp_init(void)
> > +static int sun9i_dt_parsing(struct resource res)  
> 
> Like I told you in the previous version, this should be _parse_dt, and
> not _dt_parsing.

Sorry, I forgot this review.

> 
> Also, I'm not sure why you are passing by copy the resource structure?
> 
> >  {
> > -	struct device_node *cpucfg_node, *sram_node, *node;
> > -	struct resource res;
> > +	struct device_node *prcm_node, *cpucfg_node, *sram_node;
> >  	int ret;
> >  
> > -	if (!of_machine_is_compatible("allwinner,sun9i-a80"))
> > -		return -ENODEV;
> > -
> > -	if (!sunxi_mc_smp_cpu_table_init())
> > -		return -EINVAL;
> > -
> > -	if (!cci_probed()) {
> > -		pr_err("%s: CCI-400 not available\n", __func__);
> > -		return -ENODEV;
> > -	}
> > -
> > -	node = of_find_compatible_node(NULL, NULL, "allwinner,sun9i-a80-prcm");
> > -	if (!node) {
> > -		pr_err("%s: PRCM not available\n", __func__);
> > +	prcm_node = of_find_compatible_node(NULL, NULL,
> > +				       "allwinner,sun9i-a80-prcm");
> > +	if (!prcm_node)
> >  		return -ENODEV;
> > -	}
> >  
> >  	/*
> >  	 * Unfortunately we can not request the I/O region for the PRCM.
> >  	 * It is shared with the PRCM clock.
> >  	 */
> > -	prcm_base = of_iomap(node, 0);
> > -	of_node_put(node);
> > +	prcm_base = of_iomap(prcm_node, 0);  
> 
> So it does more a bit more than just parsing the DT?
> 
> Can you maybe just fill the DT nodes and have the common part map the
> memory regions if the pointer is not NULL?

Yep, I will rework this function to only parse device tree nodes.

Thanks,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com