From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Pankaj Dubey <pankaj.dubey@samsung.com>,
linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: kgene.kim@samsung.com, thomas.ab@samsung.com, amitdanielk@gmail.com
Subject: Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver
Date: Fri, 06 Nov 2015 09:47:37 +0900 [thread overview]
Message-ID: <563BF8A9.5040708@samsung.com> (raw)
In-Reply-To: <563AE9C7.4030701@samsung.com>
On 05.11.2015 14:31, Pankaj Dubey wrote:
> Hi Krzysztof,
>
> On Tuesday 03 November 2015 07:52 AM, Krzysztof Kozlowski wrote:
>
>>
>> 1. Please reorder the exynos_sys_powerdown_conf() to be after the
>> statics. I am thinking also about adding EXPORT_SYMBOL... but maybe this
>> would be over-thinking.
>>
>
> I could not understand your point of reordering, will you please explain
> this.
Usually static functions are put at beginning of a file and the
externally linkable ones (including exportable) are at the end. This
allows quick finding of what is exported by this unit.
Mixing static-nonstatic-static brings confusion/
This is a driver so everything except it is static, so I see two choices:
1. Put it in front, just after pmu_context definition.
2. Put it in back, just before the probe.
>
>> 2. I think the proper location of everything is drivers/power/reset/.
>> Although I don't have strong opinion.
>>
>
> There has been discussion about the proper location for this driver,
> initial attempt was done in "drivers/mfd" folder but then we realized
> that this driver is not exactly fitting in MFD category.
> There was suggestion from Catalin Marinas [1], [2] to move it to
> "drivers/power" or a more suitable place other than mfd. As I received
> comments from Bartlomiej [3] and other members also (sorry I could not
> produce all links as it was quite more than a year back), I feel driver
> is very much SoC specific and hence decided to move it here.
>
> 1: https://lkml.org/lkml/2014/4/28/879
> 2:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/252018.html
>
> 3:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244690.html
In drivers/power/reset there are already very-SoC-specific reset
handlers. All of them are non-reusable outside of some SoC family.
However I don't think it really matters - both locations (soc and power)
seem fine to me.
Looking at Bart's comments I see that you did not resolve all of them.
One was left:
"what happens if exynos_sys_powerdown_conf() is called
while there are no platform devices binded to a driver but driver itself
is loaded."
Looking at the code NULL pointer exception will happen on pmu_context
dereference.
Best regards,
Krzysztof
next prev parent reply other threads:[~2015-11-06 0:47 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-26 12:55 [PATCH v3 0/7] samsung: pmu: split up SoC specific PMU data Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 1/7] ARM: EXYNOS: removing redundant code from regs-pmu.h Pankaj Dubey
2015-11-03 1:37 ` Krzysztof Kozlowski
2015-11-05 5:27 ` Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 2/7] ARM: EXYNOS: Move pmu specific headers under "linux/soc/samsung" Pankaj Dubey
2015-11-03 1:46 ` Krzysztof Kozlowski
2015-11-05 5:28 ` Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 3/7] ARCH: EXYNOS: split up exynos3250 SoC specific PMU data Pankaj Dubey
2015-11-03 1:55 ` Krzysztof Kozlowski
2015-11-05 5:31 ` Pankaj Dubey
2015-11-06 0:35 ` Krzysztof Kozlowski
2015-10-26 12:55 ` [PATCH v3 4/7] ARCH: EXYNOS: split up exynos4 " Pankaj Dubey
2015-11-03 1:56 ` Krzysztof Kozlowski
2015-11-05 5:33 ` Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 5/7] ARCH: EXYNOS: split up exynos5250 " Pankaj Dubey
2015-11-03 2:07 ` Krzysztof Kozlowski
2015-11-05 5:31 ` Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 6/7] ARCH: EXYNOS: split up exynos5420 " Pankaj Dubey
2015-11-03 2:10 ` Krzysztof Kozlowski
2015-11-05 5:31 ` Pankaj Dubey
2015-10-26 12:55 ` [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver Pankaj Dubey
2015-11-03 2:22 ` Krzysztof Kozlowski
2015-11-05 5:31 ` Pankaj Dubey
2015-11-06 0:47 ` Krzysztof Kozlowski [this message]
2015-11-03 2:06 ` [PATCH v3 0/7] samsung: pmu: split up SoC specific PMU data Krzysztof Kozlowski
2015-11-05 5:27 ` Pankaj Dubey
2015-11-06 0:18 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=563BF8A9.5040708@samsung.com \
--to=k.kozlowski@samsung.com \
--cc=amitdanielk@gmail.com \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=pankaj.dubey@samsung.com \
--cc=thomas.ab@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).