* [PATCH v5 4/4] [media] exynos-scaler: Add DT bindings for SCALER driver
From: Tomasz Figa @ 2014-01-24 16:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389238094-19386-5-git-send-email-shaik.ameer@samsung.com>
Hi Shaik,
On 09.01.2014 04:28, Shaik Ameer Basha wrote:
> This patch adds the DT binding documentation for the
> Exynos5420/5410 based SCALER device driver.
>
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/media/exynos5-scaler.txt | 22 ++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/exynos5-scaler.txt
>
> diff --git a/Documentation/devicetree/bindings/media/exynos5-scaler.txt b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
> new file mode 100644
> index 0000000..9328e7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
> @@ -0,0 +1,22 @@
> +* Samsung Exynos5 SCALER device
> +
> +SCALER is used for scaling, blending, color fill and color space
> +conversion on EXYNOS[5420/5410] SoCs.
> +
> +Required properties:
> +- compatible: should be "samsung,exynos5420-scaler" or
> + "samsung,exynos5410-scaler"
> +- reg: should contain SCALER physical address location and length
> +- interrupts: should contain SCALER interrupt number
s/number/specifier/
> +- clocks: should contain the SCALER clock specifier, from the
> + common clock bindings
s/specifier/phandle and specifier pair for each clock listed in
clock-names property/
s/from/according to/
> +- clock-names: should be "scaler"
should contain exactly one entry:
- "scaler" - IP bus clock.
Also this patch should be first in the series to let the driver added in
further patches use already present bindings.
Best regards,
Tomasz
^ permalink raw reply
* [PATCH 10/20] ARM64 / ACPI: Enumerate possible/present CPU set and map logical cpu id to APIC id
From: Hanjun Guo @ 2014-01-24 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124153557.GB23274@e102568-lin.cambridge.arm.com>
On 2014?01?24? 23:35, Lorenzo Pieralisi wrote:
> On Fri, Jan 24, 2014 at 02:37:28PM +0000, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 2014?01?22? 23:53, Lorenzo Pieralisi wrote:
>>> On Fri, Jan 17, 2014 at 12:25:04PM +0000, Hanjun Guo wrote:
>>>
>>> [...]
>>>
>>>> +/* map logic cpu id to physical GIC id */
>>>> +extern int arm_cpu_to_apicid[NR_CPUS];
>>>> +#define cpu_physical_id(cpu) arm_cpu_to_apicid[cpu]
>>> Sudeep already commented on this, please update it accordingly.
>> Actually after some careful review of the ACPI code, I can't
>> update it as MPIDR here.
>>
>> MPIDR can be the ACPI uid and NOT the GIC id, the mapping
>> of them are something like this in ACPI driver now:
>>
>> logic cpu id <---> APIC Id (GIC ID) <---> ACPI uid (MPIDR on ARM)
>> but not logic cpu id <---> ACPI uid directly, you can refer to
>> the code of processor_core.c
>>
>> So here I can only map GIC id to logical cpu id.
> On ARM platforms GIC CPU IF id is probeable, you do not need to parse
> it (ie it is not information that you will find in DT). Please have a look
> at drivers/irqchip/irq-gic.c.
>
> We have to understand what's really required and when in ACPI, or to put it
> differently, why cpu_physical_id(cpu) is required and at what time at
> boot, I will have a look on my side too.
Me too :)
>
>>>> +
>>>> #else /* !CONFIG_ACPI */
>>>> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
>>>> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
>>>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
>>>> index 8ba3e6f..1d9b789 100644
>>>> --- a/drivers/acpi/plat/arm-core.c
>>>> +++ b/drivers/acpi/plat/arm-core.c
>>>> @@ -31,6 +31,7 @@
>>>> #include <linux/smp.h>
>>>>
>>>> #include <asm/pgtable.h>
>>>> +#include <asm/cputype.h>
>>>>
>>>> /*
>>>> * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
>>>> @@ -52,6 +53,13 @@ EXPORT_SYMBOL(acpi_pci_disabled);
>>>> */
>>>> static u64 acpi_lapic_addr __initdata;
>>> Is this variable actually needed ?
>> Yes, needed for GIC initialization.
>>
>>>>
>>>> +/* available_cpus here means enabled cpu in MADT */
>>>> +static int available_cpus;
>>> Ditto.
>>>
>>>> +
>>>> +/* Map logic cpu id to physical GIC id (physical CPU id). */
>>>> +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 };
>>>> +static int boot_cpu_apic_id = -1;
>>> Do we need all these variables ? I think we should reuse cpu_logical_map
>>> data structures for that, it looks suspiciously familiar.
>> MPIDR is the different part. if we use MPIDR as GIC id, i think
>> we can reuse cpu_logical_map, but Sudeep suggested not
>> use MPIDR as GIC id.
> It is not about *reusing* cpu_logical_map, it is about setting it up
> properly. cpu_logical_map must be initialized by ACPI for the spin table
> method to work properly (and PSCI too).
>
> And yes, cpu_physical_id(cpu) is expected to be the GIC CPU IF id on
> ARM, at least it looks like, I had a look too. But this does not change
> anything as far as cpu_logical_map is concerned, it must contain a list
> of MPIDRs in the system and must be retrieved via ACPI, not DT CPU nodes
> when ACPI is used for booting.
>
> I will have a further look, since this discrepancy is annoying.
Thank you for doing this, I will look that too.
>
> [...]
>
>>>> +
>>>> + available_cpus++;
>>>> +
>>> Is available_cpus != num_possible_cpus() ? It does not look like hence
>>> available_cpus can go.
>> No, possible cpus include available cpus and disabled cpus
>> this is useful for ACPI based CPU hot-plug features.
>>
>>>> + /* allocate a logic cpu id for the new comer */
>>>> + if (boot_cpu_apic_id == id) {
>>>> + /*
>>>> + * boot_cpu_init() already hold bit 0 in cpu_present_mask
>>>> + * for BSP, no need to allocte again.
>>>> + */
>>>> + cpu = 0;
>>>> + } else {
>>>> + cpu = cpumask_next_zero(-1, cpu_present_mask);
>>>> + }
>>>> +
>>>> + /* map the logic cpu id to APIC id */
>>>> + arm_cpu_to_apicid[cpu] = id;
>>>> +
>>>> + set_cpu_present(cpu, true);
>>>> + set_cpu_possible(cpu, true);
>>> This is getting nasty. Before adding this patch and previous ones we
>>> need to put in place a method for the kernel to make a definite choice between
>>> ACPI and DT and stick to that. We can't initialize the logical map twice
>>> (which will happen if your DT has valid cpu nodes and a chosen node pointing
>>> to proper ACPI tables) or even having some entries initialized from DT and
>>> others by ACPI. It is a big fat no-no, please update the series accordingly.
>> really good catch here :)
>> so the problem here is that should we use both ACPI and DT in one system?
I think Mark had a clear answer about this :) (Answer for my self)
>>
>>
>>>> +
>>>> + return cpu;
>>>> +}
>>>> +
>>>> static int __init
>>>> acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>>>> {
>>>> @@ -144,6 +201,16 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>>>>
>>>> acpi_table_print_madt_entry(header);
>>>>
>>>> + /*
>>>> + * We need to register disabled CPU as well to permit
>>>> + * counting disabled CPUs. This allows us to size
>>>> + * cpus_possible_map more accurately, to permit
>>>> + * to not preallocating memory for all NR_CPUS
>>>> + * when we use CPU hotplug.
>>>> + */
>>>> + acpi_register_gic_cpu_interface(processor->gic_id,
>>>> + processor->flags & ACPI_MADT_ENABLED);
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> @@ -186,6 +253,19 @@ static int __init acpi_parse_madt_gic_entries(void)
>>>> return count;
>>>> }
>>>>
>>>> +#ifdef CONFIG_SMP
>>>> + if (available_cpus == 0) {
>>>> + pr_info(PREFIX "Found 0 CPUs; assuming 1\n");
>>>> + arm_cpu_to_apicid[available_cpus] =
>>>> + read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>>>> + available_cpus = 1; /* We've got at least one of these */
>>>> + }
>>> I'd rather check the MADT for at least the boot cpu to present, if it is
>>> not ACPI tables are horribly buggy and the kernel should barf on that.
>>>
>>>> +#endif
>>>> +
>>>> + /* Make boot-up look pretty */
>>>> + pr_info("%d CPUs available, %d CPUs total\n", available_cpus,
>>>> + total_cpus);
>>> Ok, now, how can we use the "disabled" CPUs == (total_cpus - available_cpus) ?
>> For cpus can be hot-added later when system is running.
> I do not see any usage in the patchset and certainly those variables are
> not used in this patch, apart from printing messages whose usefulness is
> debatable. If, as you say, you are using those variables for something
> else, please add code in the patch where they are introduced for it to be
> self-contained and to simplify the review.
ah, yes. although my ACPI based CPU hot-plug patch is ready, but need
this patch set goes in first and then send them out.
I agree with you, will try to update this patch.
Thanks
Hanjun
^ permalink raw reply
* [PATCH] arm64: Add CONFIG_CC_STACKPROTECTOR
From: Will Deacon @ 2014-01-24 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E16E42.5040000@codeaurora.org>
On Thu, Jan 23, 2014 at 07:32:18PM +0000, Laura Abbott wrote:
> On 1/23/2014 11:23 AM, Nicolas Pitre wrote:
> > Did gcc for Aarch64 replicate the same global variable arrangement?
> > That would be unfortunate...
> >
>
> Based on my experiments they did unfortunately do so.
Yes, and speaking to the tools guys, it would require an new ABI to fix
this. Given that I don't expect to see any UP AArch64 cores, I'd say drop
the #ifndef SMP part from this patch.
Will
^ permalink raw reply
* [PATCH] ARM: hw_breakpoint: Add ARMv8 support
From: Will Deacon @ 2014-01-24 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390415365-17230-1-git-send-email-cov@codeaurora.org>
Hi Christopher,
On Wed, Jan 22, 2014 at 06:29:25PM +0000, Christopher Covington wrote:
> Add the trivial support necessary to get hardware breakpoints
> working for GDB on ARMv8 simulators running in AArch32 mode.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
> arch/arm/include/asm/hw_breakpoint.h | 1 +
> arch/arm/kernel/hw_breakpoint.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
> index eef55ea..8e427c7 100644
> --- a/arch/arm/include/asm/hw_breakpoint.h
> +++ b/arch/arm/include/asm/hw_breakpoint.h
> @@ -51,6 +51,7 @@ static inline void decode_ctrl_reg(u32 reg,
> #define ARM_DEBUG_ARCH_V7_ECP14 3
> #define ARM_DEBUG_ARCH_V7_MM 4
> #define ARM_DEBUG_ARCH_V7_1 5
> +#define ARM_DEBUG_ARCH_V8 6
>
> /* Breakpoint */
> #define ARM_BREAKPOINT_EXECUTE 0
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index 3d44660..45fbcaf 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -257,6 +257,7 @@ static int enable_monitor_mode(void)
> break;
> case ARM_DEBUG_ARCH_V7_ECP14:
> case ARM_DEBUG_ARCH_V7_1:
> + case ARM_DEBUG_ARCH_V8:
> ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN));
> isb();
> break;
I'm slightly uneasy about this, mainly because I've not spent much time
looking at the AArch32 side of ARMv8 debug and I know that a fair amount
has changed since 7.1. Furthermore, the arch/arm/ kernel hasn't grown any v8
features yet and it's not clear whether that's a road down which people wish
to take it.
Anyway, assuming this works, can you please update debug_exception_updates_fsr
to return true for ARMv8 cores?
Will
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Hanjun Guo @ 2014-01-24 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124151513.GD19052@arm.com>
On 2014?01?24? 23:15, Catalin Marinas wrote:
> On Fri, Jan 24, 2014 at 12:08:15PM +0000, Mark Rutland wrote:
>> On Fri, Jan 24, 2014 at 12:20:46AM +0000, Hanjun Guo wrote:
>>> On 2014?01?22? 19:45, Mark Rutland wrote:
>>>> On Wed, Jan 22, 2014 at 08:26:50AM +0000, Linus Walleij wrote:
>>>>> On Fri, Jan 17, 2014 at 1:25 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>>>>>
>>>>>> From: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>>
>>>>>> This macro does the same job as CLOCKSOURCE_OF_DECLARE. The device
>>>>>> name from the ACPI timer table is matched with all the registered
>>>>>> timer controllers and matching initialisation routine is invoked.
>>>>>>
>>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>>> Actually I have a fat patch renaming CLOCKSOURCE_OF_DECLARE()
>>>>> to TIMER_OF_DECLARE() and I think this macro, if needed, should
>>>>> be named TIMER_ACPI_DECLARE().
>>>>>
>>>>> The reason is that "clocksource" is a Linux-internal name and this
>>>>> macro pertains to the hardware name in respective system
>>>>> description type.
>>>>>
>>>>>> +#ifdef CONFIG_ACPI
>>>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn) \
>>>>>> + static const struct acpi_device_id __clksrc_acpi_table_##name \
>>>>>> + __used __section(__clksrc_acpi_table) \
>>>>>> + = { .id = compat, \
>>>>>> + .driver_data = (kernel_ulong_t)fn }
>>>>>> +#else
>>>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn)
>>>>>> +#endif
>>>>> This hammers down the world to compile one binary for ACPI
>>>>> and one binary for device tree. Maybe that's fine, I don't know.
>>>> How does it do that?
>>>>
>>>> As far as I could tell CONFIG_ACPI and CONFIG_OF are not mutually
>>>> exclusive, and this just means that we only build the datastructures for
>>>> matching from ACPI when CONFIG_ACPI is enabled.
>>>>
>>>> Have I missed something?
>>>>
>>>> I definitely don't want to see mutually exclusive ACPI and DT support.
>>> ACPI and DT did the same job so I think they should mutually exclusive.
>>> if we enable both DT and ACPI in one system, this will leading confusions.
>> ACPI and DT do similar jobs, and we should be mutually exclusive at
>> runtime. However, they should not be mutually exclusive at compile-time.
>>
>> Being mutually exclusive at compile-time is just broken. It creates more
>> work for distributions (who need to ship double the number of kernels),
>> it increases the number of configurations requiring testing, and it
>> makes it easier for bugs to be introduced. It's just painful, and
>> there's no reason for it.
> I fully agree (IOW, I'll NAK patches that break this assumption; we want
> single kernel image whether it uses DT or ACPI).
I will not break this in next version, because I totally agree with Mark
too :)
>
>> At boot time the kernel needs to decide which to use for hardware
>> description, and completely ignore the other (which should not be
>> present, but lets not assume that or inevitably someone will break that
>> assumption for a quick hack).
>>
>> The same kernel should boot on a system that has a DTB or a system that
>> has ACPI tables. On a system that's provided both it should use one or
>> the other, but not both.
> Do we still need the chosen node to be passed via DT for command line,
> even if the kernel uses ACPI?
It depends, but I would prefer not. I prefer UEFI+ACPI and then we don't
need
the chosen node to be passed via DT.
Thanks
Hanjun
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Hanjun Guo @ 2014-01-24 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124123259.GI814@e106331-lin.cambridge.arm.com>
Hi Mark,
On 2014?01?24? 20:32, Mark Rutland wrote:
> On Fri, Jan 24, 2014 at 12:12:24AM +0000, Hanjun Guo wrote:
>> Hi Linus,
>>
>> Sorry for the late reply.
>>
>> On 2014?01?22? 16:26, Linus Walleij wrote:
>>> On Fri, Jan 17, 2014 at 1:25 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>>>
>>>> From: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>
>>>> This macro does the same job as CLOCKSOURCE_OF_DECLARE. The device
>>>> name from the ACPI timer table is matched with all the registered
>>>> timer controllers and matching initialisation routine is invoked.
>>>>
>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>> Actually I have a fat patch renaming CLOCKSOURCE_OF_DECLARE()
>>> to TIMER_OF_DECLARE() and I think this macro, if needed, should
>>> be named TIMER_ACPI_DECLARE().
>>>
>>> The reason is that "clocksource" is a Linux-internal name and this
>>> macro pertains to the hardware name in respective system
>>> description type.
>> That make sense to me too, I will update in next version if
>> this patch is still needed.
>>
>>>> +#ifdef CONFIG_ACPI
>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn) \
>>>> + static const struct acpi_device_id __clksrc_acpi_table_##name \
>>>> + __used __section(__clksrc_acpi_table) \
>>>> + = { .id = compat, \
>>>> + .driver_data = (kernel_ulong_t)fn }
>>>> +#else
>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn)
>>>> +#endif
>>> This hammers down the world to compile one binary for ACPI
>>> and one binary for device tree. Maybe that's fine, I don't know.
>> This is a problem we can have some discussion on it.
>> I prefer mutually exclusive ACPI and DT support.
> A lot of work has been put into making a single kernel boot everywhere.
> It's forced duplicated code to be factored out, and it's made the kernel
> more flexible. While it has been painful, it's forced a far higher
> quality standard across the board(s).
>
> Having a separate ACPI-capable or DT-capable kernels goes completely
> against that, and it's completely broken:
>
> * It doubles the testing effort required for a particular kernel. I can
> guarantee that we will miss bugs (even amazingly bad build bugs)
> because no-one will be able to test a full suite of kernels.
>
> * It introduces the possibility of completely pointles arbitrary
> differences between the two. How long until we see the first bug-fix
> that only works in one configuration?
>
> * It creates additional work for distributions, which need to build more
> kernels test them, distribute them, and document which platforms which
> kernels are supported on. This creates more pain for end-users too.
>
> Eventually we _will_ get fed up with all of those, and we'll have to do
> painful invasive work to make the kernel decide at runtime.
>
> Having separate kernels is a lazy shortcut. It's painful for everyone,
> leads to a greater maintenance overhead, it's not what we want now and
> not what we want in future.
>
> No thanks.
>
> Either the kernel figures out whether or not to deal with ACPI at
> runtime, or it doesn't deal with it at all.
I fully agree with you for the single kernel image, I didn't notice this
before, sorry for
my noise about the exclusive ACPI and DT support.
Thank you very much to let things much more clearer :)
Hanjun
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Mark Rutland @ 2014-01-24 15:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124151513.GD19052@arm.com>
On Fri, Jan 24, 2014 at 03:15:13PM +0000, Catalin Marinas wrote:
> Do we still need the chosen node to be passed via DT for command line,
> even if the kernel uses ACPI?
It depends on how the kernel is booted.
If not booted as a UEFI application, then the user would have to provide
a dtb to pass the command line.
If booted as a UEFI application, the kernel can retrieve the commandline
from UEFI and handle it appropriately (this _might_ involve the EFI stub
building a dtb with a chosen node and pasing that to the kernel, but
that could change as it's within the kernel). In that case, the user
need not build a dtb manually.
However, I'd expect that we use a restricted dtb with only a /chosen
node for passing additional data as an equivalent to x86's bootargs
(which would contain the location of ACPI tables and possibly other
stuff like initrd location if not passed on the command line). That
makes the very early logic in the kernel much simpler, and it gives us
an extensible format that should limit headaches in future.
So if booted with ACPI we _might_ have a restricted chosen-only dtb,
which _might_ be passed to the kernel explicitly by the user.
Thanks,
Mark.
^ permalink raw reply
* [Q] L1_CACHE_BYTES on flush_pfn_alias function.
From: Catalin Marinas @ 2014-01-24 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <00d501cf136a$24ec49c0$6ec4dd40$@samsung.com>
On Fri, Jan 17, 2014 at 09:54:42AM +0000, ?????? wrote:
> Follow the mailing-list
> http://comments.gmane.org/gmane.linux.ports.arm.omap/31686
>
> >>Setting the L1 cache line size larger than it actually is should be safe.
>
> the written code expected as L1_CACHE_BYTES should be real cache line size
> has bug.
> It looks like that flush_pfn_alias function should be fixed.
Did you actually notice any problem with flushing some more bytes? It's
a clean+invalidate rather than invalidate, I don't see any problem.
--
Catalin
^ permalink raw reply
* [PATCH] arm64: delete non-required instances of <linux/init.h>
From: Paul Gortmaker @ 2014-01-24 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124152308.GE19052@arm.com>
On 14-01-24 10:23 AM, Catalin Marinas wrote:
> On Thu, Jan 23, 2014 at 06:26:51PM +0000, Paul Gortmaker wrote:
>> None of these files are actually using any __init type directives
>> and hence don't need to include <linux/init.h>. Most are just a
>> left over from __devinit and __cpuinit removal, or simply due to
>> code getting copied from one place to the next.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>
>> [Build tested on today's linux-next ; thanks to Will for the pointer
>> to a prebuilt toolchain. Patch to be added to init cleanup series:
>> http://git.kernel.org/cgit/linux/kernel/git/paulg/init.git ]
>
> I assume you are pushing this upstream as part of your cleanup series.
Yes, that is the plan -- ack added, thanks.
P.
--
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
^ permalink raw reply
* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
From: Ian Campbell @ 2014-01-24 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2874F.10703@redhat.com>
On Fri, 2014-01-24 at 16:31 +0100, Hans de Goede wrote:
> Hi,
>
> On 01/24/2014 03:14 PM, Ian Campbell wrote:
> > I had to spend a couple of minutes proving to myself that this was the case on
> > cubietruck, so add a comment to save the next guy some effort.
>
> Seems like a good idea to me, one small nitpick though see comments inline.
>
> > Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-sunxi at googlegroups.com
> > ---
> > This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
> >
> > A plausible alternative would be to pull the memory node out of the dtsi files
> > and into the board specific files. I didn't go straight to that since I'd have
> > to research all the various boards ;-)
>
> That won't help, some boards ie the original cubieboard and the mele-a1000 come
> in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
> 2 different dts files just for that.
I didn't mean to suggest that u-boot should stop updating the dtb, so
e.g. I would have put a 512MB reg in the cubieboard dts, with a similar
comment.
But in cases where there is only one variant (e.g. the cubietruck) I'd
have put a 2GB reg and no comment, even though u-boot would in reality
rewrite the 2GB there.
> > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> > index 3e60883..9ba0beb 100644
> > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > @@ -30,6 +30,7 @@
> > };
> >
> > memory {
> > + /* 1GB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x80000000>;
> > };
> >
>
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.
Oops, I must have missed that.
>
> > diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > index 0376c50..d12ed7e 100644
> > --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
> > +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > @@ -27,6 +27,7 @@
> > };
> >
> > memory {
> > + /* 512MB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x20000000>;
> > };
> >
>
> This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
> sheet claims 2GB max RAM.
I had expected these values to contain the minimum -- i.e. the one which
would work everywhere, so it would work if for some reason u-boot wasn't
updating correctly. Better to boot with only some of the RAM than to
crash randomly.
> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index 907cfcc..658e74b 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -38,6 +38,7 @@
> > };
> >
> > memory {
> > + /* 1GB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x80000000>;
> > };
>
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.
Huh, that's the second time I got the wrong one, and I swear I was
paying attention to it as well!
Ian.
^ permalink raw reply
* [PATCH] arm64: fix build error if DMA_CMA is enabled
From: Catalin Marinas @ 2014-01-24 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390551788-6138-1-git-send-email-pankaj.dubey@samsung.com>
On Fri, Jan 24, 2014 at 08:23:08AM +0000, Pankaj Dubey wrote:
> arm64/include/asm/dma-contiguous.h is trying to include
> <asm-genric/dma-contiguous.h> which does not exist, and thus failing
> build for arm64 if we enable CONFIG_DMA_CMA. This patch fixes build
> error by removing unwanted header inclusion from arm64's dma-contiguous.h.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Somraj Mani <somraj.mani@samsung.com>
> ---
> arch/arm64/include/asm/dma-contiguous.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h
> index d6aacb6..14c4c0c 100644
> --- a/arch/arm64/include/asm/dma-contiguous.h
> +++ b/arch/arm64/include/asm/dma-contiguous.h
> @@ -18,7 +18,6 @@
> #ifdef CONFIG_DMA_CMA
>
> #include <linux/types.h>
> -#include <asm-generic/dma-contiguous.h>
Thanks for this.
Laura, did you have additional patches adding
asm-generic/dma-contiguous.h?
--
Catalin
^ permalink raw reply
* [PATCH 10/20] ARM64 / ACPI: Enumerate possible/present CPU set and map logical cpu id to APIC id
From: Lorenzo Pieralisi @ 2014-01-24 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E27AA8.8040105@linaro.org>
On Fri, Jan 24, 2014 at 02:37:28PM +0000, Hanjun Guo wrote:
> Hi Lorenzo,
>
> On 2014?01?22? 23:53, Lorenzo Pieralisi wrote:
> > On Fri, Jan 17, 2014 at 12:25:04PM +0000, Hanjun Guo wrote:
> >
> > [...]
> >
> >> +/* map logic cpu id to physical GIC id */
> >> +extern int arm_cpu_to_apicid[NR_CPUS];
> >> +#define cpu_physical_id(cpu) arm_cpu_to_apicid[cpu]
> > Sudeep already commented on this, please update it accordingly.
>
> Actually after some careful review of the ACPI code, I can't
> update it as MPIDR here.
>
> MPIDR can be the ACPI uid and NOT the GIC id, the mapping
> of them are something like this in ACPI driver now:
>
> logic cpu id <---> APIC Id (GIC ID) <---> ACPI uid (MPIDR on ARM)
> but not logic cpu id <---> ACPI uid directly, you can refer to
> the code of processor_core.c
>
> So here I can only map GIC id to logical cpu id.
On ARM platforms GIC CPU IF id is probeable, you do not need to parse
it (ie it is not information that you will find in DT). Please have a look
at drivers/irqchip/irq-gic.c.
We have to understand what's really required and when in ACPI, or to put it
differently, why cpu_physical_id(cpu) is required and at what time at
boot, I will have a look on my side too.
> >
> >> +
> >> #else /* !CONFIG_ACPI */
> >> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
> >> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
> >> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
> >> index 8ba3e6f..1d9b789 100644
> >> --- a/drivers/acpi/plat/arm-core.c
> >> +++ b/drivers/acpi/plat/arm-core.c
> >> @@ -31,6 +31,7 @@
> >> #include <linux/smp.h>
> >>
> >> #include <asm/pgtable.h>
> >> +#include <asm/cputype.h>
> >>
> >> /*
> >> * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
> >> @@ -52,6 +53,13 @@ EXPORT_SYMBOL(acpi_pci_disabled);
> >> */
> >> static u64 acpi_lapic_addr __initdata;
> > Is this variable actually needed ?
>
> Yes, needed for GIC initialization.
>
> >
> >>
> >> +/* available_cpus here means enabled cpu in MADT */
> >> +static int available_cpus;
> > Ditto.
> >
> >> +
> >> +/* Map logic cpu id to physical GIC id (physical CPU id). */
> >> +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 };
> >> +static int boot_cpu_apic_id = -1;
> > Do we need all these variables ? I think we should reuse cpu_logical_map
> > data structures for that, it looks suspiciously familiar.
>
> MPIDR is the different part. if we use MPIDR as GIC id, i think
> we can reuse cpu_logical_map, but Sudeep suggested not
> use MPIDR as GIC id.
It is not about *reusing* cpu_logical_map, it is about setting it up
properly. cpu_logical_map must be initialized by ACPI for the spin table
method to work properly (and PSCI too).
And yes, cpu_physical_id(cpu) is expected to be the GIC CPU IF id on
ARM, at least it looks like, I had a look too. But this does not change
anything as far as cpu_logical_map is concerned, it must contain a list
of MPIDRs in the system and must be retrieved via ACPI, not DT CPU nodes
when ACPI is used for booting.
I will have a further look, since this discrepancy is annoying.
[...]
> >> +
> >> + available_cpus++;
> >> +
> > Is available_cpus != num_possible_cpus() ? It does not look like hence
> > available_cpus can go.
>
> No, possible cpus include available cpus and disabled cpus
> this is useful for ACPI based CPU hot-plug features.
>
> >
> >> + /* allocate a logic cpu id for the new comer */
> >> + if (boot_cpu_apic_id == id) {
> >> + /*
> >> + * boot_cpu_init() already hold bit 0 in cpu_present_mask
> >> + * for BSP, no need to allocte again.
> >> + */
> >> + cpu = 0;
> >> + } else {
> >> + cpu = cpumask_next_zero(-1, cpu_present_mask);
> >> + }
> >> +
> >> + /* map the logic cpu id to APIC id */
> >> + arm_cpu_to_apicid[cpu] = id;
> >> +
> >> + set_cpu_present(cpu, true);
> >> + set_cpu_possible(cpu, true);
> > This is getting nasty. Before adding this patch and previous ones we
> > need to put in place a method for the kernel to make a definite choice between
> > ACPI and DT and stick to that. We can't initialize the logical map twice
> > (which will happen if your DT has valid cpu nodes and a chosen node pointing
> > to proper ACPI tables) or even having some entries initialized from DT and
> > others by ACPI. It is a big fat no-no, please update the series accordingly.
>
> really good catch here :)
> so the problem here is that should we use both ACPI and DT in one system?
>
>
> >
> >> +
> >> + return cpu;
> >> +}
> >> +
> >> static int __init
> >> acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
> >> {
> >> @@ -144,6 +201,16 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
> >>
> >> acpi_table_print_madt_entry(header);
> >>
> >> + /*
> >> + * We need to register disabled CPU as well to permit
> >> + * counting disabled CPUs. This allows us to size
> >> + * cpus_possible_map more accurately, to permit
> >> + * to not preallocating memory for all NR_CPUS
> >> + * when we use CPU hotplug.
> >> + */
> >> + acpi_register_gic_cpu_interface(processor->gic_id,
> >> + processor->flags & ACPI_MADT_ENABLED);
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -186,6 +253,19 @@ static int __init acpi_parse_madt_gic_entries(void)
> >> return count;
> >> }
> >>
> >> +#ifdef CONFIG_SMP
> >> + if (available_cpus == 0) {
> >> + pr_info(PREFIX "Found 0 CPUs; assuming 1\n");
> >> + arm_cpu_to_apicid[available_cpus] =
> >> + read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> >> + available_cpus = 1; /* We've got at least one of these */
> >> + }
> > I'd rather check the MADT for at least the boot cpu to present, if it is
> > not ACPI tables are horribly buggy and the kernel should barf on that.
> >
> >> +#endif
> >> +
> >> + /* Make boot-up look pretty */
> >> + pr_info("%d CPUs available, %d CPUs total\n", available_cpus,
> >> + total_cpus);
> > Ok, now, how can we use the "disabled" CPUs == (total_cpus - available_cpus) ?
>
> For cpus can be hot-added later when system is running.
I do not see any usage in the patchset and certainly those variables are
not used in this patch, apart from printing messages whose usefulness is
debatable. If, as you say, you are using those variables for something
else, please add code in the patch where they are introduced for it to be
self-contained and to simplify the review.
Thanks,
Lorenzo
^ permalink raw reply
* [PATCH 03/20] ARM64 / ACPI: Introduce the skeleton of _PDC related for ARM64
From: Hanjun Guo @ 2014-01-24 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123180335.GK27520@arm.com>
Hi Catalin,
On 2014?01?24? 02:03, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 12:24:57PM +0000, Hanjun Guo wrote:
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Copyright (C) 2013, Al Stone <al.stone@linaro.org>
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + */
>> +
>> +#ifndef _ASM_ARM64_ACPI_H
>> +#define _ASM_ARM64_ACPI_H
> Nitpick: please use __ASM_ACPI_H as a guard for consistency (the same
> for all the other patches introducing new header files).
ok, will update this patch.
Hanjun
>
^ permalink raw reply
* [PATCH] ARM-i.MX6Q-dts : Added USB_OTG Support
From: Mark Rutland @ 2014-01-24 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5B=2mYWeoj1snSMpDzxHK8H3_DKevuaA4T4bXiV=64vkg@mail.gmail.com>
On Fri, Jan 24, 2014 at 12:15:08PM +0000, Fabio Estevam wrote:
> Hi Mark,
>
> On Fri, Jan 24, 2014 at 9:50 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> >> +
> >> + regulators {
> >> + compatible = "simple-bus";
> >
> > This is _not_ a simple bus. It doesn't have the required ranges
> > property.
> >
> > Why do these need to be in a regulators container node? We don't group
> > dma controllers under a dmas node, or uarts under a uarts node.
>
> It seems we have this same issue on several imx6 dts files.
>
> Would the below address your suggestion?
The below patch looks good to me.
In general there seems to be a misunderstanding of the purpose of
simple-bus, as an annotation that children of a node should be probed,
rather than as a representation of a simple bus (which is of the same
type as the parent-bus, which requires no programming, where children
can be used without knowledge of the simple-bus).
There seem to be a lot of cases where simple-bus is used as a fallback
compatible string, when in reality the node's children make no sense
without information from and/or programming of the node with the
simple-bus property. Those cases are completely wrong, and a complete
abuse of simple-bus.
There are other cases where simple-bus nodes are used to group nodes
from a commonly reused block. As long as these have the requisite
ranges, #address-cells, and #size-cells, then they are valid, and make
sense for translating / widening addresses for common groups of
peripherals even if they're not on a different physical bus.
I don't see the point in grouping together nodes in an artificial
container because of their functionality rather than their topology, it
seems to breed confusion.
Cheers,
Mark.
>
> diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-
> index e75e11b..ba35560 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> @@ -15,33 +15,29 @@
> reg = <0x10000000 0x40000000>;
> };
>
> - regulators {
> - compatible = "simple-bus";
> -
> - reg_usb_otg_vbus: usb_otg_vbus {
> - compatible = "regulator-fixed";
> - regulator-name = "usb_otg_vbus";
> - regulator-min-microvolt = <5000000>;
> - regulator-max-microvolt = <5000000>;
> - gpio = <&gpio3 22 0>;
> - enable-active-high;
> - };
> + reg_usb_otg_vbus: regulator at 0 {
> + compatible = "regulator-fixed";
> + regulator-name = "usb_otg_vbus";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio3 22 0>;
> + enable-active-high;
> + };
>
> - reg_usb_h1_vbus: usb_h1_vbus {
> - compatible = "regulator-fixed";
> - regulator-name = "usb_h1_vbus";
> - regulator-min-microvolt = <5000000>;
> - regulator-max-microvolt = <5000000>;
> - gpio = <&gpio1 29 0>;
> - enable-active-high;
> - };
> + reg_usb_h1_vbus: regulator at 1 {
> + compatible = "regulator-fixed";
> + regulator-name = "usb_h1_vbus";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio1 29 0>;
> + enable-active-high;
> + };
>
> - reg_audio: wm8962_supply {
> - compatible = "regulator-fixed";
> - regulator-name = "wm8962-supply";
> - gpio = <&gpio4 10 0>;
> - enable-active-high;
> - };
> + reg_audio: regulator at 2 {
> + compatible = "regulator-fixed";
> + regulator-name = "wm8962-supply";
> + gpio = <&gpio4 10 0>;
> + enable-active-high;
> };
>
> gpio-keys {
>
> If so, I will prepare some patches to update other dts files.
>
> Thanks,
>
> Fabio Estevam
>
^ permalink raw reply
* [PATCH 09/20] ARM64 / ACPI: Implement core functions for parsing MADT table
From: Hanjun Guo @ 2014-01-24 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E15756.4060705@arm.com>
Hi Marc,
On 2014?01?24? 01:54, Marc Zyngier wrote:
> Hi Hanjun,
>
> On 17/01/14 12:25, Hanjun Guo wrote:
>> Implement core functions for parsing MADT table to get the information
>> about GIC cpu interface and GIC distributor to prepare for SMP and GIC
>> initialization.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>> arch/arm64/include/asm/acpi.h | 3 +
>> drivers/acpi/plat/arm-core.c | 139 ++++++++++++++++++++++++++++++++++++++++-
>> drivers/acpi/tables.c | 21 +++++++
>> 3 files changed, 162 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index e108d9c..c335c6d 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -83,6 +83,9 @@ void arch_fix_phys_package_id(int num, u32 slot);
>> extern int (*acpi_suspend_lowlevel)(void);
>> #define acpi_wakeup_address (0)
>>
>> +#define MAX_GIC_CPU_INTERFACE 256
> I'll bite. Where on Earth is this value coming from?
I just thought 256 is big enough for now :(
Yes, should be a larger number for GICv3.
> If that's for
> GICv2, 8 is the maximum. For GICv3+, that's incredibly low, and should
> be probed probed at runtime anyway.
I would prefer to do that, but this value is used to
probe CPUs in MADT :)
>
>> +#define MAX_GIC_DISTRIBUTOR 1 /* should be the same as MAX_GIC_NR */
> No support for cascaded GICs?
Yes, no cascade GICs in ACPI at now.
>
>> +
>> #else /* !CONFIG_ACPI */
>> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
>> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
>> index 1835b21..8ba3e6f 100644
>> --- a/drivers/acpi/plat/arm-core.c
>> +++ b/drivers/acpi/plat/arm-core.c
>> @@ -46,6 +46,16 @@ EXPORT_SYMBOL(acpi_disabled);
>> int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */
>> EXPORT_SYMBOL(acpi_pci_disabled);
>>
>> +/*
>> + * Local interrupt controller address,
>> + * GIC cpu interface base address on ARM/ARM64
>> + */
>> +static u64 acpi_lapic_addr __initdata;
> If that's a GIC address, why not call it as such?
thanks for the suggesting, I will update.
>
>> +#define BAD_MADT_ENTRY(entry, end) ( \
>> + (!entry) || (unsigned long)entry + sizeof(*entry) > end || \
>> + ((struct acpi_subtable_header *)entry)->length < sizeof(*entry))
>> +
>> #define PREFIX "ACPI: "
> Just do:
> #define pr_fmt(fmt) "ACPI: " fmt
>
> and remove all the occurrences of PREFIX.
>
>> /* FIXME: this function should be moved to topology.c when it is ready */
>> @@ -92,6 +102,115 @@ void __init __acpi_unmap_table(char *map, unsigned long size)
>> return;
>> }
>>
>> +static int __init acpi_parse_madt(struct acpi_table_header *table)
>> +{
>> + struct acpi_table_madt *madt = NULL;
> No need to initialize this to NULL, you're doing an assignment at the
> next line...
>
>> +
>> + madt = (struct acpi_table_madt *)table;
>> + if (!madt) {
>> + pr_warn(PREFIX "Unable to map MADT\n");
> There is no mapping here, please fix the message accordingly.
Ok, I will address your comments above in next version.
>
>> + return -ENODEV;
>> + }
>> +
>> + if (madt->address) {
>> + acpi_lapic_addr = (u64) madt->address;
> So you're updating this static variable, for the distributor and each
> CPU interface? /me puzzled...
Good catch. So I have a question: do we really have some SoCs
without banked registers on ARM64? if not , I think we can use
a single static variable is ok.
>
>> + pr_info(PREFIX "Local APIC address 0x%08x\n", madt->address);
> Away with this APIC madness. GICC and GICD are the concepts we're all
> familiar with here, and using the proper terminology would certainly
> help reviewing these patches...
That make sense to me too, will update.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * GIC structures on ARM are somthing like Local APIC structures on x86,
>> + * which means GIC cpu interfaces for GICv2/v3. Every GIC structure in
>> + * MADT table represents a cpu in the system.
> And what do you do when your GICv3 doesn't have a memory-mapped
> interface, but only uses system registers?
>
>> + * GIC distributor structures are somthing like IOAPIC on x86. GIC can
>> + * be initialized with information in this structure.
>> + *
>> + * Please refer to chapter5.2.12.14/15 of ACPI 5.0
> A pointer to that documentation?
Please refer to http://www.acpi.info/
>
>> + */
>> +
>> +static int __init
>> +acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>> +{
>> + struct acpi_madt_generic_interrupt *processor = NULL;
>> +
>> + processor = (struct acpi_madt_generic_interrupt *)header;
>> +
>> + if (BAD_MADT_ENTRY(processor, end))
>> + return -EINVAL;
>> +
>> + acpi_table_print_madt_entry(header);
>> +
>> + return 0;
>> +}
>> +
>> +static int __init
>> +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
>> + const unsigned long end)
>> +{
>> + struct acpi_madt_generic_distributor *distributor = NULL;
>> +
>> + distributor = (struct acpi_madt_generic_distributor *)header;
>> +
>> + if (BAD_MADT_ENTRY(distributor, end))
>> + return -EINVAL;
>> +
>> + acpi_table_print_madt_entry(header);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Parse GIC cpu interface related entries in MADT
>> + * returns 0 on success, < 0 on error
>> + */
>> +static int __init acpi_parse_madt_gic_entries(void)
>> +{
>> + int count;
>> +
>> + /*
>> + * do a partial walk of MADT to determine how many CPUs
>> + * we have including disabled CPUs
>> + */
>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
>> + acpi_parse_gic, MAX_GIC_CPU_INTERFACE);
>> +
>> + if (!count) {
>> + pr_err(PREFIX "No GIC entries present\n");
>> + return -ENODEV;
>> + } else if (count < 0) {
>> + pr_err(PREFIX "Error parsing GIC entry\n");
>> + return count;
>> + }
> So you do a lot of parsing to count stuff, and then discard the number
> of counted objects... You might as well check that there is at least one
> valid object and stop there.
>
>> + return 0;
>> +}
>> +
>> +/*
>> + * Parse GIC distributor related entries in MADT
>> + * returns 0 on success, < 0 on error
>> + */
>> +static int __init acpi_parse_madt_gic_distributor_entries(void)
>> +{
>> + int count;
>> +
>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>> + acpi_parse_gic_distributor, MAX_GIC_DISTRIBUTOR);
>> +
>> + if (!count) {
>> + pr_err(PREFIX "No GIC distributor entries present\n");
>> + return -ENODEV;
>> + } else if (count < 0) {
>> + pr_err(PREFIX "Error parsing GIC distributor entry\n");
>> + return count;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>> {
>> *irq = gsi_to_irq(gsi);
>> @@ -141,11 +260,29 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
>>
>> static void __init early_acpi_process_madt(void)
>> {
>> - return;
>> + acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt);
>> }
>>
>> static void __init acpi_process_madt(void)
>> {
>> + int error;
>> +
>> + if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) {
> How many times are you going to parse the same table? Surely you can
> stash whatever information you need and be done with it?
good catch, we already addressed this problem, and will
update in next version.
>
>> + /*
>> + * Parse MADT GIC cpu interface entries
>> + */
>> + error = acpi_parse_madt_gic_entries();
>> + if (!error) {
>> + /*
>> + * Parse MADT GIC distributor entries
>> + */
>> + acpi_parse_madt_gic_distributor_entries();
>> + }
>> + }
>> +
>> + pr_info("Using ACPI for processor (GIC) configuration information\n");
>> +
>> return;
>> }
>>
>> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
>> index d67a1fe..b3e4615 100644
>> --- a/drivers/acpi/tables.c
>> +++ b/drivers/acpi/tables.c
>> @@ -191,6 +191,27 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
>> }
>> break;
>>
>> + case ACPI_MADT_TYPE_GENERIC_INTERRUPT:
>> + {
>> + struct acpi_madt_generic_interrupt *p =
>> + (struct acpi_madt_generic_interrupt *)header;
>> + printk(KERN_INFO PREFIX
> Use pr_info
this function use printk always, should change all of them?
>
>> + "GIC (acpi_id[0x%04x] gic_id[0x%04x] %s)\n",
>> + p->uid, p->gic_id,
>> + (p->flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
>> + }
>> + break;
>> +
>> + case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR:
>> + {
>> + struct acpi_madt_generic_distributor *p =
>> + (struct acpi_madt_generic_distributor *)header;
>> + printk(KERN_INFO PREFIX
>> + "GIC Distributor (id[0x%04x] address[0x%08llx] gsi_base[%d])\n",
>> + p->gic_id, p->base_address, p->global_irq_base);
>> + }
>> + break;
>> +
>> default:
>> printk(KERN_WARNING PREFIX
>> "Found unsupported MADT entry (type = 0x%x)\n",
>>
> Most of that code seems to be repeatedly parsing and printing stuff, and
> I fail to see what it actually does.
yes, just print some information when booting.
Thank you very much for the comments.
Hanjun
^ permalink raw reply
* [PATCH] ARM-i.MX6Q-dts : Added USB_OTG Support
From: Shawn Guo @ 2014-01-24 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124115005.GG814@e106331-lin.cambridge.arm.com>
On Fri, Jan 24, 2014 at 11:50:05AM +0000, Mark Rutland wrote:
> > @@ -18,6 +18,19 @@
> > memory {
> > reg = <0x10000000 0x80000000>;
> > };
> > +
> > + regulators {
> > + compatible = "simple-bus";
>
> This is _not_ a simple bus. It doesn't have the required ranges
> property.
Maybe the real question is that if the required ranges property is
missing, why kernel still accepts it as a simple bus and populate the
devices under it?
>
> Why do these need to be in a regulators container node? We don't group
> dma controllers under a dmas node, or uarts under a uarts node.
Grouping fixed clocks under 'clocks' and fixed regulators under
'regulators' is a common pattern which can be found on most of the ARM
dts files.
Shawn
>
> > +
> > + reg_usb_otg_vbus: usb_otg_vbus {
> > + compatible = "regulator-fixed";
> > + regulator-name = "usb_otg_vbus";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + gpio = <&gpio4 15 0>;
> > + enable-active-low;
> > + };
> > + };
> > };
>
> Thanks,
> Mark.
^ permalink raw reply
* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
From: Hans de Goede @ 2014-01-24 15:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390572852-15027-1-git-send-email-ijc@hellion.org.uk>
Hi,
On 01/24/2014 03:14 PM, Ian Campbell wrote:
> I had to spend a couple of minutes proving to myself that this was the case on
> cubietruck, so add a comment to save the next guy some effort.
Seems like a good idea to me, one small nitpick though see comments inline.
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-sunxi at googlegroups.com
> ---
> This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
>
> A plausible alternative would be to pull the memory node out of the dtsi files
> and into the board specific files. I didn't go straight to that since I'd have
> to research all the various boards ;-)
That won't help, some boards ie the original cubieboard and the mele-a1000 come
in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
2 different dts files just for that.
>
> I considered maing it "reg = <0 0>" but decided that having some sort of least
> common denominator would let things work even if the bootloader were broken.
> ---
> arch/arm/boot/dts/sun4i-a10.dtsi | 1 +
> arch/arm/boot/dts/sun5i-a10s.dtsi | 1 +
> arch/arm/boot/dts/sun5i-a13.dtsi | 1 +
> arch/arm/boot/dts/sun6i-a31.dtsi | 1 +
> arch/arm/boot/dts/sun7i-a20.dtsi | 1 +
> 5 files changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 3e60883..9ba0beb 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -30,6 +30,7 @@
> };
>
> memory {
> + /* 1GB by default, will be updated by U-Boot */
> reg = <0x40000000 0x80000000>;
> };
>
The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
datasheet claims as max RAM.
> diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
> index 0376c50..d12ed7e 100644
> --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
> +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
> @@ -27,6 +27,7 @@
> };
>
> memory {
> + /* 512MB by default, will be updated by U-Boot */
> reg = <0x40000000 0x20000000>;
> };
>
This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
sheet claims 2GB max RAM.
> diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
> index b81aeb9..6f8bfd9 100644
> --- a/arch/arm/boot/dts/sun5i-a13.dtsi
> +++ b/arch/arm/boot/dts/sun5i-a13.dtsi
> @@ -27,6 +27,7 @@
> };
>
> memory {
> + /* 512MB by default, will be updated by U-Boot */
> reg = <0x40000000 0x20000000>;
> };
>
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index 3f6f07b..bcbef9a 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -46,6 +46,7 @@
> };
>
> memory {
> + /* 2GB by default, will be updated by U-Boot */
> reg = <0x40000000 0x80000000>;
> };
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 907cfcc..658e74b 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -38,6 +38,7 @@
> };
>
> memory {
> + /* 1GB by default, will be updated by U-Boot */
> reg = <0x40000000 0x80000000>;
> };
The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
datasheet claims as max RAM.
Regards,
Hans
^ permalink raw reply
* [PATCH v2 3/7] devicetree: bindings: add cpu clock configuration data binding for Exynos4/5
From: Thomas Abraham @ 2014-01-24 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJuA9ai+5+t=WKvvxK7MzriAFd2fBXOVVt9YgZ18XQ5a9mr4Og@mail.gmail.com>
Hi Rob,
On Tue, Jan 21, 2014 at 1:01 PM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> Hi Rob,
>
>
> On Sat, Jan 18, 2014 at 8:52 PM, Rob Herring <robherring2@gmail.com> wrote:
>> On Sat, Jan 18, 2014 at 6:10 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>>> From: Thomas Abraham <thomas.ab@samsung.com>
>>>
>>> The clk ops of the new Samsung cpu clock provider type requires configuration
>>> data that will be programmed in the multiple clock blocks encapsulated within
>>> the cpu clock provider type. This configuration data is held in the clock
>>> controller node. Update clock binding documentation about this configuration
>>> data format for Samsung Exynos4 and Exynos5 platforms.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>
>> Please copy all maintainers.
>
> Okay.
>
>>
>>> Cc: Tomasz Figa <t.figa@samsung.com>
>>> Cc: <devicetree@vger.kernel.org>
>>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>>> ---
>>> .../devicetree/bindings/clock/exynos4-clock.txt | 30 ++++++++++++++++++++
>>> .../devicetree/bindings/clock/exynos5250-clock.txt | 21 ++++++++++++++
>>> 2 files changed, 51 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> index a2ac2d9..c28aabd 100644
>>> --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> @@ -15,6 +15,29 @@ Required Properties:
>>>
>>> - #clock-cells: should be 1.
>>>
>>> +- arm-frequency-table: defines the list of arm clock speeds supported and
>>
>> Seems like a Samsung specific property and nothing to do with ARM, so
>> it should be named accordingly.
>
> Okay.
>
>>
>>> + the associated configuration values required to setup the clock controller
>>> + for generating those speeds. The format of each entry included in the
>>> + arm-frequency-table should be as defined below (#cells per entry = 13)
>>> +
>>> + - for Exynos4210 and Exynos4212 based platforms:
>>> + cell #1: arm clock frequency
>>> + cell #2: expected arm clock parent frequency
>>> + cell #3 ~ cell 12#: value of clock divider in the following order
>>> + core_ratio, corem0_ratio, corem1_ratio, periph_ratio,
>>> + atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio,
>>> + copy_ratio, hpm_ratio.
>>> + cell #13: reserved (should be zero).
>>> +
>>> + - for Exynos4412 based platforms:
>>> + cell #1: arm clock frequency
>>> + cell #2: expected arm clock parent frequency
>>> + cell #3 ~ cell #13: value of clock divider in the following order
>>> + core_ratio, corem0_ratio, corem1_ratio, periph_ratio,
>>> + atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio,
>>> + copy_ratio, hpm_ratio, cores_ratio
>>
>> You don't need voltages? Are the h/w limitations really ratios or each
>> clock has a max frequency that must be maintained? I would expect the
>> latter and think it would be better to specify maximum frequencies of
>> each clock. Then you can calculate the dividers to keep frequencies in
>> range.
>
> Voltage is not needed here since the cpu clock speed is managed by the
> cpufreq driver which will take care of setting appropriate voltage.
> Any other users apart from cpufreq will have to setup the voltage
> appropriately.
>
> About the ratios, yes the user manual lists the optimal clock
> frequency for these clocks. As suggested, I will remove the ratio list
> for these clocks.
I tried the approach of listing the optimal frequencies of these
clocks in dt and then calculating a divider value. But it turns out
that the divider values for these clocks is dependent on the frequency
of the PLL from which they are derived (or the voltage applied to the
CPU clock domain). During tests, the system is unstable if divider
values are calculated using the optimal clock speed listed in the SoC
manual.
So the list of divider values as used in this series (and documented
above) is required. Can I continue to use the binding as documented
above?
Thanks,
Thomas.
>
>>
>> How will this scale to multi-cluster chips with different frequency ranges?
>
> Usually, each cluster would have its own independent clock input. So
> there will be two or more cpu clocks in multi-cluster chips. So the
> clocks can be independently programmed as required by the cluster.
>
>>
>> Rob
>>
>
> Thanks Rob for your review.
>
> Thomas.
>
>>> +
>>> +
>>> The following is the list of clocks generated by the controller. Each clock is
>>> assigned an identifier and client nodes use this identifier to specify the
>>> clock which they consume. Some of the clocks are available only on a particular
>>> @@ -275,6 +298,13 @@ Example 1: An example of a clock controller node is listed below.
>>> compatible = "samsung,exynos4210-clock";
>>> reg = <0x10030000 0x20000>;
>>> #clock-cells = <1>;
>>> +
>>> + arm-frequency-table = <1200000 1200000 0 3 7 3 4 1 7 0 5 0>,
>>> + <1000000 1000000 0 3 7 3 4 1 7 0 4 0>,
>>> + < 800000 800000 0 3 7 3 3 1 7 0 3 0>,
>>> + < 500000 500000 0 3 7 3 3 1 7 0 3 0>,
>>> + < 400000 400000 0 3 7 3 3 1 7 0 3 0>,
>>> + < 200000 200000 0 1 3 1 1 1 0 0 3 0>;
>>> };
>>>
>>> Example 2: UART controller node that consumes the clock generated by the clock
>>> diff --git a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> index 72ce617..99eae9c 100644
>>> --- a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> +++ b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> @@ -13,6 +13,20 @@ Required Properties:
>>>
>>> - #clock-cells: should be 1.
>>>
>>> +- arm-frequency-table: defines the list of arm clock speeds supported and
>>> + the associated configuration values required to setup the clock controller
>>> + for generating those speeds. The format of each entry included in the
>>> + arm-frequency-table should be as defined below (#cells per entry = 13)
>>> +
>>> + cell #1: arm clock frequency
>>> + cell #2: expected arm clock parent frequency
>>> + cell #3 ~ cell #12: value of clock divider in the following order
>>> + arm_ratio, cpud_ratio, acp_ratio, periph_ratio,
>>> + atb_ratio, pclk_dbg_ratio, apll_ratio, arm2_ratio,
>>> + copy_ratio, hpm_ratio
>>> + cell #13: reserved (should be zero)
>>> +
>>> +
>>> The following is the list of clocks generated by the controller. Each clock is
>>> assigned an identifier and client nodes use this identifier to specify the
>>> clock which they consume.
>>> @@ -177,6 +191,13 @@ Example 1: An example of a clock controller node is listed below.
>>> compatible = "samsung,exynos5250-clock";
>>> reg = <0x10010000 0x30000>;
>>> #clock-cells = <1>;
>>> +
>>> + arm-frequency-table = <1700000 1700000 0 3 7 7 7 3 5 0 0 2>,
>>> + <1600000 1600000 0 3 7 7 7 1 4 0 0 2>,
>>> + <1500000 1500000 0 2 7 7 7 1 4 0 0 2>,
>>> + <1400000 1400000 0 2 7 7 6 1 4 0 0 2>,
>>> + <1300000 1300000 0 2 7 7 6 1 3 0 0 2>,
>>> + <1200000 1200000 0 2 7 7 5 1 3 0 0 2>;
>>> };
>>>
>>> Example 2: UART controller node that consumes the clock generated by the clock
>>> --
>>> 1.6.6.rc2
>>>
^ permalink raw reply
* [PATCH] arm64: delete non-required instances of <linux/init.h>
From: Catalin Marinas @ 2014-01-24 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390501611-31691-1-git-send-email-paul.gortmaker@windriver.com>
On Thu, Jan 23, 2014 at 06:26:51PM +0000, Paul Gortmaker wrote:
> None of these files are actually using any __init type directives
> and hence don't need to include <linux/init.h>. Most are just a
> left over from __devinit and __cpuinit removal, or simply due to
> code getting copied from one place to the next.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>
> [Build tested on today's linux-next ; thanks to Will for the pointer
> to a prebuilt toolchain. Patch to be added to init cleanup series:
> http://git.kernel.org/cgit/linux/kernel/git/paulg/init.git ]
I assume you are pushing this upstream as part of your cleanup series.
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Catalin Marinas @ 2014-01-24 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124120815.GH814@e106331-lin.cambridge.arm.com>
On Fri, Jan 24, 2014 at 12:08:15PM +0000, Mark Rutland wrote:
> On Fri, Jan 24, 2014 at 12:20:46AM +0000, Hanjun Guo wrote:
> > On 2014?01?22? 19:45, Mark Rutland wrote:
> > > On Wed, Jan 22, 2014 at 08:26:50AM +0000, Linus Walleij wrote:
> > >> On Fri, Jan 17, 2014 at 1:25 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
> > >>
> > >>> From: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > >>>
> > >>> This macro does the same job as CLOCKSOURCE_OF_DECLARE. The device
> > >>> name from the ACPI timer table is matched with all the registered
> > >>> timer controllers and matching initialisation routine is invoked.
> > >>>
> > >>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > >>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> > >> Actually I have a fat patch renaming CLOCKSOURCE_OF_DECLARE()
> > >> to TIMER_OF_DECLARE() and I think this macro, if needed, should
> > >> be named TIMER_ACPI_DECLARE().
> > >>
> > >> The reason is that "clocksource" is a Linux-internal name and this
> > >> macro pertains to the hardware name in respective system
> > >> description type.
> > >>
> > >>> +#ifdef CONFIG_ACPI
> > >>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn) \
> > >>> + static const struct acpi_device_id __clksrc_acpi_table_##name \
> > >>> + __used __section(__clksrc_acpi_table) \
> > >>> + = { .id = compat, \
> > >>> + .driver_data = (kernel_ulong_t)fn }
> > >>> +#else
> > >>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn)
> > >>> +#endif
> > >> This hammers down the world to compile one binary for ACPI
> > >> and one binary for device tree. Maybe that's fine, I don't know.
> > > How does it do that?
> > >
> > > As far as I could tell CONFIG_ACPI and CONFIG_OF are not mutually
> > > exclusive, and this just means that we only build the datastructures for
> > > matching from ACPI when CONFIG_ACPI is enabled.
> > >
> > > Have I missed something?
> > >
> > > I definitely don't want to see mutually exclusive ACPI and DT support.
> >
> > ACPI and DT did the same job so I think they should mutually exclusive.
> > if we enable both DT and ACPI in one system, this will leading confusions.
>
> ACPI and DT do similar jobs, and we should be mutually exclusive at
> runtime. However, they should not be mutually exclusive at compile-time.
>
> Being mutually exclusive at compile-time is just broken. It creates more
> work for distributions (who need to ship double the number of kernels),
> it increases the number of configurations requiring testing, and it
> makes it easier for bugs to be introduced. It's just painful, and
> there's no reason for it.
I fully agree (IOW, I'll NAK patches that break this assumption; we want
single kernel image whether it uses DT or ACPI).
> At boot time the kernel needs to decide which to use for hardware
> description, and completely ignore the other (which should not be
> present, but lets not assume that or inevitably someone will break that
> assumption for a quick hack).
>
> The same kernel should boot on a system that has a DTB or a system that
> has ACPI tables. On a system that's provided both it should use one or
> the other, but not both.
Do we still need the chosen node to be passed via DT for command line,
even if the kernel uses ACPI?
--
Catalin
^ permalink raw reply
* [PATCH 0/2] DT updates for Hummingboard and new Cubox-i
From: Shawn Guo @ 2014-01-24 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119185246.GB28056@quad.lixom.net>
On Sun, Jan 19, 2014 at 10:52:46AM -0800, Olof Johansson wrote:
> I'm well aware that the DT branch has not been merged, and that's because
> there has been no consensus from DT maintainers on the massive rework
> you did as the base of the branch.
I guess the 'massive rework' you're talking about is the patch series
that begins with 'ARM: dts: imx6qdl: make pinctrl nodes board specific'?
If so, I hope you would agree that we're trying to solve a real
problem [1], even though the solution does not look like the best to
you.
So which specific part of the solution are you objecting to? Those
MX6QDL_*_PINGRP macros defined in imx6qdl-pingrp.h? If that's the
case, I can send a follow-up patch to kill the macros by filling in the
pin group definitions directly. The pros is that the pin group
definitions for given board will be more intuitive, and the cons is that
the change set will be even more massive, because the same multi-lines
pin group definitions will be duplicated in multiple board dts files,
which use the same group of pins for given device.
Or any other better idea?
>
> Given the timing, it's not looking like it will be resolved in time for
> the 3.14 merge window (which might open today).
That's frustrating and will be surprising a lot of people who expect to
see their board support in 3.14 release.
Shawn
[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/
^ permalink raw reply
* [PATCH 11/20] ARM64 / ACPI: Get the enable method for SMP initialization
From: Hanjun Guo @ 2014-01-24 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123175031.GJ27520@arm.com>
Hi Catalin,
On 2014?01?24? 01:50, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 12:25:05PM +0000, Hanjun Guo wrote:
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -48,6 +48,7 @@
>> #include <asm/sections.h>
>> #include <asm/tlbflush.h>
>> #include <asm/ptrace.h>
>> +#include <asm/acpi.h>
>>
>> /*
>> * as from 2.5, kernels no longer have an init_tasks structure
>> @@ -280,7 +281,7 @@ static void (*smp_cross_call)(const struct cpumask *, unsigned int);
>> * cpu logical map array containing MPIDR values related to logical
>> * cpus. Assumes that cpu_logical_map(0) has already been initialized.
>> */
>> -void __init smp_init_cpus(void)
>> +static int __init of_smp_init_cpus(void)
>> {
>> struct device_node *dn = NULL;
>> unsigned int i, cpu = 1;
>> @@ -364,6 +365,10 @@ next:
>> cpu++;
>> }
>>
>> + /* No device tree or no CPU node in DT */
>> + if (cpu == 1 && !bootcpu_valid)
>> + return -ENODEV;
>> +
>> /* sanity check */
>> if (cpu > NR_CPUS)
>> pr_warning("no. of cores (%d) greater than configured maximum of %d - clipping\n",
>> @@ -371,7 +376,7 @@ next:
>>
>> if (!bootcpu_valid) {
>> pr_err("DT missing boot CPU MPIDR, not enabling secondaries\n");
>> - return;
>> + return -EINVAL;
>> }
>>
>> /*
>> @@ -381,6 +386,39 @@ next:
>> for (i = 0; i < NR_CPUS; i++)
>> if (cpu_logical_map(i) != INVALID_HWID)
>> set_cpu_possible(i, true);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * In ACPI mode, the cpu possible map was enumerated before SMP
>> + * initialization when MADT table was parsed, so we can get the
>> + * possible map here to initialize CPUs.
>> + */
>> +static void __init acpi_smp_init_cpus(void)
>> +{
>> + int cpu;
>> + const char *enable_method;
>> +
>> + for_each_possible_cpu(cpu) {
>> + enable_method = acpi_get_enable_method(cpu);
>> + if (!enable_method)
>> + continue;
>> +
>> + cpu_ops[cpu] = cpu_get_ops(enable_method);
>> + if (!cpu_ops[cpu])
>> + continue;
>> +
>> + cpu_ops[cpu]->cpu_init(NULL, cpu);
>> + }
>> +}
>> +
>> +void __init smp_init_cpus(void)
>> +{
>> + if (!of_smp_init_cpus())
>> + return;
>> +
>> + acpi_smp_init_cpus();
>> }
> With DT we initialise the cpu_ops[0] via cpu_read_bootcpu_ops() called
> from setup_arch(). This is needed because with PSCI we use cpu_ops for
> power management even if it's a UP system. Do you get a some kernel
> warning about device node for the boot cpu not found?
Thanks for pointing this out, actually we didn't find a dts file with
spin-table method for SMP initialization, and this patch is not fully
tested (only spin-table method is supported in ACPI now), we are
working on that and get this patch fully tested.
I will review the code carefully in your comments, and update
the code accordingly.
>
>> --- a/drivers/acpi/plat/arm-core.c
>> +++ b/drivers/acpi/plat/arm-core.c
>> @@ -367,6 +367,32 @@ static void __init acpi_process_madt(void)
>> }
>>
>> /*
>> + * To see PCSI is enabled or not.
>> + *
>> + * PSCI is not available for ACPI 5.0, return FALSE for now.
>> + *
>> + * FIXME: should we introduce early_param("psci", func) for test purpose?
>> + */
>> +static bool acpi_psci_smp_available(int cpu)
>> +{
>> + return FALSE;
>> +}
>> +
>> +static const char *enable_method[] = {
>> + "psci",
>> + "spin-table",
>> + NULL
>> +};
>> +
>> +const char *acpi_get_enable_method(int cpu)
>> +{
>> + if (acpi_psci_smp_available(cpu))
>> + return enable_method[0];
>> + else
>> + return enable_method[1];
>> +}
> You could just use literal strings here, actually even ignoring PSCI
> until available.
Ok.
Thanks
Hanjun
^ permalink raw reply
* [PATCH 07/20] ARM64 / ACPI: Enable ARM64 in Kconfig
From: Hanjun Guo @ 2014-01-24 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123163939.GB25927@localhost>
On 2014?01?24? 00:39, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 12:25:01PM +0000, Hanjun Guo wrote:
>> Add Kconfigs to build ACPI on ARM64, and make ACPI runable on ARM64.
>>
>> acpi_idle driver is x86/IA64 dependent now, so make CONFIG_ACPI_PROCESSOR
>> depends on X86 || IA64, and implement it on ARM/ARM64 in the furture.
>>
>> In order to make arm-core.c can both run on ARM and ARM64, introduce
>> CONFIG_ACPI_ARM to support it.
>>
>> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Signed-off-by: Al Stone <al.stone@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>> arch/arm64/Kconfig | 2 ++
>> drivers/acpi/Kconfig | 11 ++++++++---
>> drivers/acpi/plat/Makefile | 2 +-
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 6d4dd22..2b1fb1d 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -279,6 +279,8 @@ source "net/Kconfig"
>>
>> source "drivers/Kconfig"
>>
>> +source "drivers/acpi/Kconfig"
>> +
>> source "fs/Kconfig"
>>
>> source "arch/arm64/kvm/Kconfig"
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 4770de5..cae5dc9 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -2,13 +2,16 @@
>> # ACPI Configuration
>> #
>>
>> +config ACPI_ARM
>> + bool
> Could be better as def_bool ARM64
>
>> +
>> menuconfig ACPI
>> bool "ACPI (Advanced Configuration and Power Interface) Support"
>> depends on !IA64_HP_SIM
>> - depends on IA64 || X86
>> - depends on PCI
>> + depends on ((IA64 || X86) && PCI) || ARM64
>> select PNP
>> - default y
>> + select ACPI_ARM if ARM64
> And remove this select here.
Thanks for the guidance, will update in next version.
>
^ permalink raw reply
* [PATCH 03/20] ARM64 / ACPI: Introduce the skeleton of _PDC related for ARM64
From: Hanjun Guo @ 2014-01-24 14:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123161918.GA25927@localhost>
On 2014?01?24? 00:19, Catalin Marinas wrote:
> On Mon, Jan 20, 2014 at 09:20:38AM +0000, Hanjun Guo wrote:
>> On 2014-1-17 22:25, Sudeep Holla wrote:
>>> On 17/01/14 12:24, Hanjun Guo wrote:
>>>> --- a/arch/arm64/kernel/process.c
>>>> +++ b/arch/arm64/kernel/process.c
>>>> @@ -89,6 +89,9 @@ void arch_cpu_idle_prepare(void)
>>>> local_fiq_enable();
>>>> }
>>>>
>>>> +unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE;
>>>> +EXPORT_SYMBOL(boot_option_idle_override);
>>>> +
>>> This is what I mentioned in other email. Do we really foresee use of this in
>>> ARM64 or it's just added to avoid build issues ?
>> Just avoid build issues, can not foresee use of this in ARM64 :)
> So ideally we should look for a better solution here.
Ok, I will try my best.
Thanks
Hanjun
^ permalink raw reply
* [PATCH 02/20] ARM64 : Add dummy asm/cpu.h
From: Hanjun Guo @ 2014-01-24 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123161505.GB23939@localhost>
On 2014?01?24? 00:15, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 12:24:56PM +0000, Hanjun Guo wrote:
>> ACPI requires a cpu.h, add a dummy one copied from arm. This will need
>> updated or replaced as ACPI based cpu hotplug or cpu topology for armv8
>> is worked out.
>>
>> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>> arch/arm64/include/asm/cpu.h | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 arch/arm64/include/asm/cpu.h
>>
>> diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
>> new file mode 100644
>> index 0000000..8625eb1
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/cpu.h
>> @@ -0,0 +1,25 @@
>> +/*
>> + * Copyright (C) 2004-2005 ARM Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#ifndef __ASM_ARM_CPU_H
>> +#define __ASM_ARM_CPU_H
>> +
>> +#include <linux/percpu.h>
>> +#include <linux/cpu.h>
>> +#include <linux/topology.h>
>> +
>> +struct cpuinfo_arm {
>> + struct cpu cpu;
>> + u64 cpuid;
>> +#ifdef CONFIG_SMP
>> + unsigned int loops_per_jiffy;
>> +#endif
>> +};
>> +
>> +DECLARE_PER_CPU(struct cpuinfo_arm, cpu_data);
>> +
>> +#endif
> Could you not leave this file empty until you add the code that actually
> makes use of cpu_data?
Of course, I will update in next version :)
Thanks
Hanjun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox