linux-kernel - Re: [PATCH 1/2] x86/quark: Add Quark embedded SRAM support

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:	Tue, 05 May 2015 10:44:47 +0200
From:	Paul Bolle <pebolle@...cali.nl>
To:	Bryan O'Donoghue <pure.logic@...us-software.ie>
Cc:	tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
	x86@...nel.org, dvhart@...radead.org, andy.schevchenko@...il.com,
	boon.leong.ong@...el.com, linux-kernel@...r.kernel.org,
	platform-driver-x86@...r.kernel.org, derek.browne@...el.com,
	josef.ahmad@...el.com, erik.nyquist@...el.com
Subject: Re: [PATCH 1/2] x86/quark: Add Quark embedded SRAM support

On Mon, 2015-05-04 at 03:17 +0100, Bryan O'Donoghue wrote:
> --- a/arch/x86/platform/intel-quark/Makefile
> +++ b/arch/x86/platform/intel-quark/Makefile

>  obj-$(CONFIG_INTEL_IMR) += imr.o

(Your change to drivers/platform/x86/Kconfig now makes it possible that
imr.o will be part of a module. More on that below.)

> +obj-$(CONFIG_INTEL_ESRAM) += esram.o

INTEL_ESRAM is a bool Kconfig symbol, so esram.o will never be part of a
module, right?

> --- /dev/null
> +++ b/arch/x86/platform/intel-quark/esram.c

> +MODULE_DEVICE_TABLE(x86cpu, esram_ids);

> +module_init(esram_init);
> +module_exit(esram_exit);
> +
> +MODULE_AUTHOR("Bryan O'Donoghue <pure.logic@...us-software.ie>");
> +MODULE_DESCRIPTION("Intel Embedded SRAM overlay driver");
> +MODULE_LICENSE("Dual BSD/GPL");
> +

(Trailing empty line.)

> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig

>  config INTEL_IMR
> -	bool "Intel Isolated Memory Region support"
> +	tristate "Intel Isolated Memory Region support"

It's not obvious, at least to me, why this symbol is changed to
tristate. Does that follow from the other changes?

>  	default n
>  	depends on X86_INTEL_QUARK && IOSF_MBI
>  	---help---
 
> +config INTEL_ESRAM
> +	bool "Intel Embedded SRAM (eSRAM) support"

If I'm reading the corresponding Makefile change correctly, esram.o will
never be part of a module. But esram.c is added with some module
specific boilerplate. (Note, again, that I'm not sure whether
KBUILD_MODNAME is module specific.)

Was your intention perhaps to make this a tristate symbol?

> +	default n
> +	depends on X86_INTEL_QUARK && IOSF_MBI
> +	select GENERIC_ALLOCATOR
> +	---help---
> +	  This options provides an API to allocate memory from Embedded SRAM
> +	  (eSRAM) present on Quark X1000 SoC processors.
> +	  eSRAM is a 512 KiB block of low-latency SRAM organized as
> +	  128 * 4 KiB pages or as one 512 KiB chunk of memory. This driver
> +	  enables eSRAM in per-page overlay mode and provides a gen_pool
> +	  allocator which allows allocation of memory from the eSRAM pool.
> +
> +	  If you are running on a Galileo/Quark say Y here.

Thanks,


Paul Bolle

--
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/