linux-kernel - Re: [PATCH v4 6/8] drivers: soc: add support for exynos SROM driver

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]

Message-id: <5625B92C.10505@samsung.com>
Date:	Tue, 20 Oct 2015 09:16:52 +0530
From:	Pankaj Dubey <pankaj.dubey@...sung.com>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Cc:	kgene.kim@...sung.com, p.fedin@...sung.com, thomas.ab@...sung.com
Subject: Re: [PATCH v4 6/8] drivers: soc: add support for exynos SROM driver

Hi Krzysztof,

On Tuesday 20 October 2015 05:40 AM, Krzysztof Kozlowski wrote:
> On 19.10.2015 20:46, Pankaj Dubey wrote:
>> This patch adds Exynos SROM controller driver which will handle
>> save restore of SROM registers during S2R.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@...sung.com>
>> ---
>>   drivers/soc/Kconfig               |   1 +
>>   drivers/soc/Makefile              |   1 +
>>   drivers/soc/samsung/Kconfig       |  13 +++
>>   drivers/soc/samsung/Makefile      |   1 +
>>   drivers/soc/samsung/exynos-srom.c | 179 ++++++++++++++++++++++++++++++++++++++
>>   drivers/soc/samsung/exynos-srom.h |  51 +++++++++++
>>   6 files changed, 246 insertions(+)
>>   create mode 100644 drivers/soc/samsung/Kconfig
>>   create mode 100644 drivers/soc/samsung/Makefile
>>   create mode 100644 drivers/soc/samsung/exynos-srom.c
>>   create mode 100644 drivers/soc/samsung/exynos-srom.h
>>
>> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
>> index 96ddecb..69107c9 100644
>> --- a/drivers/soc/Kconfig
>> +++ b/drivers/soc/Kconfig
>> @@ -2,6 +2,7 @@ menu "SOC (System On Chip) specific Drivers"
>>
>>   source "drivers/soc/mediatek/Kconfig"
>>   source "drivers/soc/qcom/Kconfig"
>> +source "drivers/soc/samsung/Kconfig"
>>   source "drivers/soc/sunxi/Kconfig"
>>   source "drivers/soc/ti/Kconfig"
>>   source "drivers/soc/versatile/Kconfig"
>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
>> index 0b12d77..a623616 100644
>> --- a/drivers/soc/Makefile
>> +++ b/drivers/soc/Makefile
>> @@ -5,6 +5,7 @@
>>   obj-$(CONFIG_MACH_DOVE)		+= dove/
>>   obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
>>   obj-$(CONFIG_ARCH_QCOM)		+= qcom/
>> +obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
>>   obj-$(CONFIG_ARCH_SUNXI)	+= sunxi/
>>   obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
>>   obj-$(CONFIG_SOC_TI)		+= ti/
>> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
>> new file mode 100644
>> index 0000000..ea4bc2a
>> --- /dev/null
>> +++ b/drivers/soc/samsung/Kconfig
>> @@ -0,0 +1,13 @@
>> +#
>> +# SAMSUNG SoC drivers
>> +#
>> +menu "Samsung SOC driver support"
>> +
>> +config SOC_SAMSUNG
>> +	bool
>> +
>> +config EXYNOS_SROM
>> +	bool
>> +	depends on ARM && ARCH_EXYNOS
>
> When !PM then the driver will... do nothing, right? So maybe make it
> depending on PM so tiny configs would benefit?
>

Yes. Currently driver will do nothing if !PM. But as we know Fedin, has 
a plan to extend this driver for auxiliary H/W IP hooked to SROM. So in 
that case this dependency will not be valid as those functionality may 
not be dependent on PM, and we may need to remove it later. So I feel 
better not to add it at first place itself.


>> +static int exynos_srom_remove(struct platform_device *pdev)
>> +{
>> +	struct exynos_srom *srom = platform_get_drvdata(pdev);
>> +
>> +	kfree(srom->reg_offset);
>> +	iounmap(srom->reg_base);
>> +	srom->reg_base = NULL;
>> +	srom->reg_offset = NULL;
>
> There is no need anymore for these two NULL-s. It made sense only in
> previous code when these were global variables. At this point the device
> callbacks cannot be accessed so NULL-ifying does not change anything.
>

Agreed. Will update.

> Rest from my point of view looks good.
>

Thanks for review.

Pankaj

> Best regards,
> Krzysztof
>
>
--
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/