linux-kernel - Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55DF1257.9090303@linaro.org>
Date:	Thu, 27 Aug 2015 21:36:23 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	Fu Wei <fu.wei@...aro.org>,
	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
	Linaro ACPI Mailman List <linaro-acpi@...ts.linaro.org>,
	linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>, linux-doc@...r.kernel.org,
	Wei Fu <tekkamanninja@...il.com>,
	G Gregory <graeme.gregory@...aro.org>,
	Al Stone <al.stone@...aro.org>, Arnd Bergmann <arnd@...db.de>,
	Guenter Roeck <linux@...ck-us.net>,
	Vipul Gandhi <vgandhi@...eaurora.org>,
	Wim Van Sebroeck <wim@...ana.be>,
	Jon Masters <jcm@...hat.com>, Leo Duran <leo.duran@....com>,
	Jonathan Corbet <corbet@....net>,
	Mark Rutland <mark.rutland@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Rafael Wysocki <rjw@...ysocki.net>, dyoung@...hat.com,
	panand@...hat.com, Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c

On 08/27/2015 08:28 PM, Hanjun Guo wrote:
> On 08/27/2015 08:08 PM, Thomas Gleixner wrote:
>> On Thu, 27 Aug 2015, Hanjun Guo wrote:
>>> On 08/26/2015 03:17 AM, Thomas Gleixner wrote:
>>>> On Wed, 26 Aug 2015, Fu Wei wrote:
>>>>>>>    /* Initialize per-processor generic timer */
>>>>>>> -static int __init arch_timer_acpi_init(struct acpi_table_header
>>>>>>> *table)
>>>>>>> +void __init arch_timer_acpi_init(void)
>>>>>>>    {
>>>>>>
>>>>>> And how is that supposed to work when we have next generation CPUs
>>>>>> which implement a different timer? You break multisystem kernels that
>>>>>> way.
>>>
>>> Sorry, I think I missed some context here that I don't understand
>>> why the code here will break multisystem kernels? I'm trying to
>>> understand the problem here and update the code :)
>>>
>>>>>
>>>>> yes, you are right, If there is a  next generation CPUs  which
>>>>> implement a different timer, (maybe) this driver can not work.
>>>>> we may need a new timer driver.
>>>>>
>>>>> But,
>>>>> (1) for now,  aarch64  core always has the arch timer(this timer is
>>>>> part of aarch64 architecture).
>>>>> and the existing code make  ARM64 kernel "select ARM_ARCH_TIMER "
>>>>> (2) GTDT is designed for generic timer, so in this call "
>>>>> arch_timer_acpi_init"  we  parse the gtdt info.
>>>>> (3) once we have a ARM64 CPUs which implement a different timer, we
>>>>> may need to select a right timer in the config stage.
>>>>> and this timer may not be described in GTDT.  So we can implement
>>>>> another arch_timer_acpi_init by that time in new timer driver..
>>>>> if the new time still uses GTDT(or new version GTDT), we may need to
>>>>> update gtdt.c for new timer by that time.
>>>>
>>>> That's simply wrong. You want to build kernels which run on both cpus
>>>> and the selection of the timer happens at runtime depending on the
>>>> ACPI info. We do the same thing with device tree.
>>>
>>> I think the code can do that if I understand correctly. The code for
>>> now is that we only support arch timer on ARM64, and this patch set
>>> is adding SBSA watchdog timer support which need same function in
>>> arch timer, so we move that function to common place.
>>>
>>> We will load the driver (arch timer, memory mapped timer) when the
>>> ACPI table defines them, which when new timer is coming, that will
>>> defined in the ACPI table and load the driver as needed.
>>>
>>> Please correct me if I misse something, thanks.
>>
>> arch_timer_acpi_init() is called from the architecture boot code. So
>> how is that supposed to work with different timers?
>>
>> Are you going to have bla_timer_acpi_init() and foo_timer_acpi_init()
>> calls as well?
>>
>> Why not having a something like DT has: DECLARE_....
>>
>> and the arch_timer_acpi_init() using that to figure out which of the
>> timers to initialize.
>
> Ah, ok, I can fully understand you now, thanks for your patience.
>
> Yes, I agree with you, so this is not a problem for this patch, but
> for the code implementation of previous code. Actually we are on the
> road to do as you suggested, we introduced something like
> #define ACPI_DECLARE(table, name, table_id, subtable, data, fn) [1]
> in the GICv3 and GIC self probe patch set, and I said that
> infrastructure can be used as clock declare too, we just trying
> to not add such dependence on that patch set (it's still on discussion),
>
> [1]: https://lkml.org/lkml/2015/7/29/236
>
> If that is ok with you, we will introduce similar DECLARE_ thing
> for clock declare.

Or we can drop this patch from this patch set, and clean up this
patch when the ACPI_DECLARE() infrastructure is ready for upstream.

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/