linux-kernel - Re: [PATCH v3 6/6] ACPI: import watchdog info of GTDT into platform device

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]

Date:	Wed, 27 May 2015 00:27:33 +0800
From:	Fu Wei <fu.wei@...aro.org>
To:	Will Deacon <will.deacon@....com>
Cc:	Guenter Roeck <linux@...ck-us.net>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
	"linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"tekkamanninja@...il.com" <tekkamanninja@...il.com>,
	"graeme.gregory@...aro.org" <graeme.gregory@...aro.org>,
	"al.stone@...aro.org" <al.stone@...aro.org>,
	"hanjun.guo@...aro.org" <hanjun.guo@...aro.org>,
	"timur@...eaurora.org" <timur@...eaurora.org>,
	"arnd@...db.de" <arnd@...db.de>,
	"vgandhi@...eaurora.org" <vgandhi@...eaurora.org>,
	"wim@...ana.be" <wim@...ana.be>, "jcm@...hat.com" <jcm@...hat.com>,
	"leo.duran@....com" <leo.duran@....com>,
	"corbet@....net" <corbet@....net>,
	Mark Rutland <Mark.Rutland@....com>,
	Catalin Marinas <Catalin.Marinas@....com>
Subject: Re: [PATCH v3 6/6] ACPI: import watchdog info of GTDT into platform device

On 26 May 2015 at 23:36, Guenter Roeck <linux@...ck-us.net> wrote:
> On Tue, May 26, 2015 at 04:18:42PM +0100, Will Deacon wrote:
>> On Tue, May 26, 2015 at 04:02:56PM +0100, Ashwin Chaugule wrote:
>> > On 26 May 2015 at 08:28, Will Deacon <will.deacon@....com> wrote:
>> > > On Mon, May 25, 2015 at 11:03:13AM +0100, fu.wei@...aro.org wrote:
>> > >> From: Fu Wei <fu.wei@...aro.org>
>> > >>
>> > >> Parse SBSA Generic Watchdog Structure in GTDT table of ACPI,
>> > >> and create a platform device with that information.
>> > >> This platform device can be used by the ARM SBSA Generic
>> > >> Watchdog driver.
>> > >>
>> > >> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
>> > >> Tested-by: Timur Tabi <timur@...eaurora.org>
>> > >> Signed-off-by: Fu Wei <fu.wei@...aro.org>
>> > >> ---
>> > >>  arch/arm64/kernel/acpi.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++
>> > >>  1 file changed, 145 insertions(+)
>> > >
>> > > Why does this all need to be under arch/arm64? The GTDT really isn't
>> > > architecture-specific, so I'd *much* rather it was parsed in the driver code
>> > > itself, like we already do for the architected timer. The GIC is an
>> > > exception because it's in the MADT, which we need to parse in the arch code
>> > > to configure SMP properly.
>> >
>> > I'm not really against refactoring the code. But the GTDT looks quite
>> > specific to ARM..
>> >
>> > ---8<----
>> > 5.2.24 Generic Timer Description Table (GTDT)
>> > This section describes the format of the Generic Timer Description
>> > Table (GTDT), which provides
>> > OSPM with information about a system’s Generic Timers configuration.
>> > The Generic Timer (GT) is
>> > a standard timer interface implemented on ARM processor-based systems.
>> > The GT hardware
>> > specification can be found at Links to ACPI-Related Documents
>> > (http://uefi.org/acpi) under the
>> > heading ARM Architecture. The GTDT provides OSPM with information
>> > about a system's GT
>> > interrupt configurations, for both per-processor timers, and platform
>> > (memory-mapped) timers.
>> > The GT specification defines the following per-processor timers:
>> > • Secure privilege level 1 (EL1) timer,
>> > • Non-Secure EL1 timer,
>> > • Non-Secure privilege level 2 (EL2) timer,
>> > • Virtual timer,
>> > and the following Platform (memory-mapped) timers.
>> > • GT Block
>> > • Server Base System Architecture (SBSA) Generic Watchdog
>> > ---8<----
>>
>> Sure, the device it describes may only ever exist on ARM systems, but by
>> that logic then we should be moving lots of drivers back under arch/arm[64].
>>
> It is nt the driver, but its instantiation. The question here would be
> how and where to instantiate the driver, not where the driver itself
> is located. The driver itself is ACPI agnostic.

Hi Will,

I really don't mind to refactor the code, If we can make this patch better.

But for now, I can't see the good reason to move ACPI-relevant code
into a watchdog driver.

The reasons I put the code here are
(1)SBSA watchdog only for ARM64
(2)GTDT only for ARM, design for ARM,
(3)For ARM Architecture, only ARM64 support ACPI.

For minimizing arch/arm64/kernel/acpi.c, we can't put the code here,
and we had better keep these code outside the driver,

So do you have any suggestion for the better location of the GTDT code?

Great thanks for your help.

>
> What you are really saying is that you want the driver instantiation
> to be moved into the driver.
>
> Guenter



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021
--
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/