From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S937702AbdAEEEU (ORCPT ); Wed, 4 Jan 2017 23:04:20 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:34415 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751283AbdAEEET (ORCPT ); Wed, 4 Jan 2017 23:04:19 -0500 Subject: Re: [PATCHv2 2/5] arm: mvebu: support for SMP on 98DX3336 SoC To: Chris Packham , linux-arm-kernel@lists.infradead.org References: <20170105033641.6212-1-chris.packham@alliedtelesis.co.nz> <20170105033641.6212-3-chris.packham@alliedtelesis.co.nz> Cc: Rob Herring , Mark Rutland , Jason Cooper , Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Russell King , Geert Uytterhoeven , Chris Brand , Sudeep Holla , Jayachandran C , Juri Lelli , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org From: Florian Fainelli Message-ID: <5edb6751-d15b-6654-f4ae-adf38fafb8a4@gmail.com> Date: Wed, 4 Jan 2017 20:04:16 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170105033641.6212-3-chris.packham@alliedtelesis.co.nz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 01/04/17 à 19:36, Chris Packham a écrit : > Compared to the armada-xp the 98DX3336 uses different registers to set > the boot address for the secondary CPU so a new enable-method is needed. > This will only work if the machine definition doesn't define an overall > smp_ops because there is not currently a way of overriding this from the > device tree if it is set in the machine definition. Not sure I follow you here, in theory, each individual CPU could have a different enable-method property, and considering how you leverage existing DTS include files, you should be able to override the DTS with the appropriate enable-method, or do you have a different problem? mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) > +{ > + WARN_ON(hw_cpu != 1); > + > + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); > + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + > + MV98DX3236_CPU_RESUME_ADDR_OFFSET); I just submitted a patch series that switches such users to __pa_symbol() instead of virt_to_phys() because this is a kernel image symbol. For now this will work as-is, but depending on which patch series makes it first, it may be a good idea to keep this on someone's TODO list (yours or mine). I do expect having to make a second pass of conversions anyway. [1]: https://lkml.org/lkml/2017/1/4/1101 > +} > + > +static int __init mv98dx3236_resume_init(void) > +{ > + struct device_node *np; > + struct resource res; > + int ret = 0; > + > + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); > + if (!np) > + return 0; > + > + pr_info("Initializing 98DX3236 Resume\n"); This can be probably be dropped, you should know fairly quickly if this succeeded or not. Can't this function be implemented as a smp_ops::smp_init_cpus instead of having this initialization done at arch_initcall time? > + > + if (of_address_to_resource(np, 0, &res)) { > + pr_err("unable to get resource\n"); > + ret = -ENOENT; > + goto out; > + } > + > + if (!request_mem_region(res.start, resource_size(&res), > + np->full_name)) { > + pr_err("unable to request region\n"); > + ret = -EBUSY; > + goto out; > + } You should be able to use of_io_request_and_map() and here. -- Florian From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCHv2 2/5] arm: mvebu: support for SMP on 98DX3336 SoC Date: Wed, 4 Jan 2017 20:04:16 -0800 Message-ID: <5edb6751-d15b-6654-f4ae-adf38fafb8a4@gmail.com> References: <20170105033641.6212-1-chris.packham@alliedtelesis.co.nz> <20170105033641.6212-3-chris.packham@alliedtelesis.co.nz> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20170105033641.6212-3-chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Chris Packham , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Cc: Rob Herring , Mark Rutland , Jason Cooper , Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Russell King , Geert Uytterhoeven , Chris Brand , Sudeep Holla , Jayachandran C , Juri Lelli , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org Le 01/04/17 à 19:36, Chris Packham a écrit : > Compared to the armada-xp the 98DX3336 uses different registers to set > the boot address for the secondary CPU so a new enable-method is needed. > This will only work if the machine definition doesn't define an overall > smp_ops because there is not currently a way of overriding this from the > device tree if it is set in the machine definition. Not sure I follow you here, in theory, each individual CPU could have a different enable-method property, and considering how you leverage existing DTS include files, you should be able to override the DTS with the appropriate enable-method, or do you have a different problem? mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) > +{ > + WARN_ON(hw_cpu != 1); > + > + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); > + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + > + MV98DX3236_CPU_RESUME_ADDR_OFFSET); I just submitted a patch series that switches such users to __pa_symbol() instead of virt_to_phys() because this is a kernel image symbol. For now this will work as-is, but depending on which patch series makes it first, it may be a good idea to keep this on someone's TODO list (yours or mine). I do expect having to make a second pass of conversions anyway. [1]: https://lkml.org/lkml/2017/1/4/1101 > +} > + > +static int __init mv98dx3236_resume_init(void) > +{ > + struct device_node *np; > + struct resource res; > + int ret = 0; > + > + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); > + if (!np) > + return 0; > + > + pr_info("Initializing 98DX3236 Resume\n"); This can be probably be dropped, you should know fairly quickly if this succeeded or not. Can't this function be implemented as a smp_ops::smp_init_cpus instead of having this initialization done at arch_initcall time? > + > + if (of_address_to_resource(np, 0, &res)) { > + pr_err("unable to get resource\n"); > + ret = -ENOENT; > + goto out; > + } > + > + if (!request_mem_region(res.start, resource_size(&res), > + np->full_name)) { > + pr_err("unable to request region\n"); > + ret = -EBUSY; > + goto out; > + } You should be able to use of_io_request_and_map() and here. -- Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: f.fainelli@gmail.com (Florian Fainelli) Date: Wed, 4 Jan 2017 20:04:16 -0800 Subject: [PATCHv2 2/5] arm: mvebu: support for SMP on 98DX3336 SoC In-Reply-To: <20170105033641.6212-3-chris.packham@alliedtelesis.co.nz> References: <20170105033641.6212-1-chris.packham@alliedtelesis.co.nz> <20170105033641.6212-3-chris.packham@alliedtelesis.co.nz> Message-ID: <5edb6751-d15b-6654-f4ae-adf38fafb8a4@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Le 01/04/17 ? 19:36, Chris Packham a ?crit : > Compared to the armada-xp the 98DX3336 uses different registers to set > the boot address for the secondary CPU so a new enable-method is needed. > This will only work if the machine definition doesn't define an overall > smp_ops because there is not currently a way of overriding this from the > device tree if it is set in the machine definition. Not sure I follow you here, in theory, each individual CPU could have a different enable-method property, and considering how you leverage existing DTS include files, you should be able to override the DTS with the appropriate enable-method, or do you have a different problem? mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) > +{ > + WARN_ON(hw_cpu != 1); > + > + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); > + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + > + MV98DX3236_CPU_RESUME_ADDR_OFFSET); I just submitted a patch series that switches such users to __pa_symbol() instead of virt_to_phys() because this is a kernel image symbol. For now this will work as-is, but depending on which patch series makes it first, it may be a good idea to keep this on someone's TODO list (yours or mine). I do expect having to make a second pass of conversions anyway. [1]: https://lkml.org/lkml/2017/1/4/1101 > +} > + > +static int __init mv98dx3236_resume_init(void) > +{ > + struct device_node *np; > + struct resource res; > + int ret = 0; > + > + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); > + if (!np) > + return 0; > + > + pr_info("Initializing 98DX3236 Resume\n"); This can be probably be dropped, you should know fairly quickly if this succeeded or not. Can't this function be implemented as a smp_ops::smp_init_cpus instead of having this initialization done at arch_initcall time? > + > + if (of_address_to_resource(np, 0, &res)) { > + pr_err("unable to get resource\n"); > + ret = -ENOENT; > + goto out; > + } > + > + if (!request_mem_region(res.start, resource_size(&res), > + np->full_name)) { > + pr_err("unable to request region\n"); > + ret = -EBUSY; > + goto out; > + } You should be able to use of_io_request_and_map() and here. -- Florian