From: Krzysztof Kozlowski <krzk@kernel.org> To: Pankaj Dubey <pankaj.dubey@samsung.com> Cc: linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, arnd@arndb.de, Marek Szyprowski <m.szyprowski@samsung.com>, kgene@kernel.org, m.reichl@fivetechno.de, a.hajda@samsung.com, cwchoi00@gmail.com, Javier Martinez Canillas <javier@osg.samsung.com> Subject: Re: [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr,flag} ops to exynos_s2r_data Date: Fri, 7 Apr 2017 12:32:48 +0200 [thread overview] Message-ID: <CAJKOXPeFhmcscCX-so5qSx9pdX8oEmC+-283HO+J3vJNvWr_Hg@mail.gmail.com> (raw) In-Reply-To: <1490879826-16754-9-git-send-email-pankaj.dubey@samsung.com> On Thu, Mar 30, 2017 at 3:17 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote: > Various Exynos SoC needs different boot addresses and flags. Currently we > are handling this difference by adding lots of soc_is_exynosMMM checks in > the code, in an attempt to remove the dependency of such helper functions > specific to each SoC, let's separate helper functions for these helper > functions by moving them into SoC specific hooks in struct exynos_s2r_data. > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > --- > arch/arm/mach-exynos/pm.c | 60 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 50 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c > index fa24098..c3fa537 100644 > --- a/arch/arm/mach-exynos/pm.c > +++ b/arch/arm/mach-exynos/pm.c > @@ -32,26 +32,56 @@ > #include "common.h" > > struct exynos_s2r_data { > + void __iomem* (*boot_vector_addr)(void); > + void __iomem* (*boot_vector_flag)(void); > void (*enter_aftr)(void); > }; OK, now I see more uses of this structure so the naming could be "exynos_pm_data"? > > static const struct exynos_s2r_data *s2r_data; > > -static inline void __iomem *exynos_boot_vector_addr(void) > +static void __iomem *exynos_boot_vector_addr(void) > +{ > + if (s2r_data && s2r_data->boot_vector_addr) > + return s2r_data->boot_vector_addr(); > + > + return NULL; > +} > + > +static inline void __iomem *exynos4210_rev11_boot_vector_addr(void) Inlines here are mixed up and you are changing them without explanation in commit msg. Previously the exynos_boot_vector_addr() was inlined, now not. Okay, I can accept that but please mention this in commit msg. But below you are adding new inline functions which are stored as pointers in ops. This looks both inconsistent with above and incorrect from logical point of view. How would you like to inline them if they are referenced through pointer? (okay, compilers are smart and crazy so maybe they can do it but anyway I am curious how this would look like). > +{ > + return pmu_base_addr + S5P_INFORM7; > +} > + > +static inline void __iomem *exynos4210_rev10_boot_vector_addr(void) > +{ > + return sysram_base_addr + 0x24; > +} > + > +static inline void __iomem *exynos_common_boot_vector_addr(void) > { > - if (samsung_rev() == EXYNOS4210_REV_1_1) > - return pmu_base_addr + S5P_INFORM7; > - else if (samsung_rev() == EXYNOS4210_REV_1_0) > - return sysram_base_addr + 0x24; > return pmu_base_addr + S5P_INFORM0; > } > > -static inline void __iomem *exynos_boot_vector_flag(void) > +static void __iomem *exynos_boot_vector_flag(void) ditto for the flag. Best regards, Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: krzk@kernel.org (Krzysztof Kozlowski) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr, flag} ops to exynos_s2r_data Date: Fri, 7 Apr 2017 12:32:48 +0200 [thread overview] Message-ID: <CAJKOXPeFhmcscCX-so5qSx9pdX8oEmC+-283HO+J3vJNvWr_Hg@mail.gmail.com> (raw) In-Reply-To: <1490879826-16754-9-git-send-email-pankaj.dubey@samsung.com> On Thu, Mar 30, 2017 at 3:17 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote: > Various Exynos SoC needs different boot addresses and flags. Currently we > are handling this difference by adding lots of soc_is_exynosMMM checks in > the code, in an attempt to remove the dependency of such helper functions > specific to each SoC, let's separate helper functions for these helper > functions by moving them into SoC specific hooks in struct exynos_s2r_data. > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > --- > arch/arm/mach-exynos/pm.c | 60 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 50 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c > index fa24098..c3fa537 100644 > --- a/arch/arm/mach-exynos/pm.c > +++ b/arch/arm/mach-exynos/pm.c > @@ -32,26 +32,56 @@ > #include "common.h" > > struct exynos_s2r_data { > + void __iomem* (*boot_vector_addr)(void); > + void __iomem* (*boot_vector_flag)(void); > void (*enter_aftr)(void); > }; OK, now I see more uses of this structure so the naming could be "exynos_pm_data"? > > static const struct exynos_s2r_data *s2r_data; > > -static inline void __iomem *exynos_boot_vector_addr(void) > +static void __iomem *exynos_boot_vector_addr(void) > +{ > + if (s2r_data && s2r_data->boot_vector_addr) > + return s2r_data->boot_vector_addr(); > + > + return NULL; > +} > + > +static inline void __iomem *exynos4210_rev11_boot_vector_addr(void) Inlines here are mixed up and you are changing them without explanation in commit msg. Previously the exynos_boot_vector_addr() was inlined, now not. Okay, I can accept that but please mention this in commit msg. But below you are adding new inline functions which are stored as pointers in ops. This looks both inconsistent with above and incorrect from logical point of view. How would you like to inline them if they are referenced through pointer? (okay, compilers are smart and crazy so maybe they can do it but anyway I am curious how this would look like). > +{ > + return pmu_base_addr + S5P_INFORM7; > +} > + > +static inline void __iomem *exynos4210_rev10_boot_vector_addr(void) > +{ > + return sysram_base_addr + 0x24; > +} > + > +static inline void __iomem *exynos_common_boot_vector_addr(void) > { > - if (samsung_rev() == EXYNOS4210_REV_1_1) > - return pmu_base_addr + S5P_INFORM7; > - else if (samsung_rev() == EXYNOS4210_REV_1_0) > - return sysram_base_addr + 0x24; > return pmu_base_addr + S5P_INFORM0; > } > > -static inline void __iomem *exynos_boot_vector_flag(void) > +static void __iomem *exynos_boot_vector_flag(void) ditto for the flag. Best regards, Krzysztof
next prev parent reply other threads:[~2017-04-07 10:32 UTC|newest] Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20170330131417epcas1p48a7f41b90177294b7918ecf31df130d1@epcas1p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 00/12] Introducing Exynos ChipId driver Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey [not found] ` <CGME20170330131419epcas5p4ac36250d3a9225643e327e60f47956c2@epcas5p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 01/12] ARM: EXYNOS: refactor firmware specific routines Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey 2017-04-07 7:45 ` Krzysztof Kozlowski 2017-04-07 7:45 ` Krzysztof Kozlowski 2017-04-07 8:06 ` pankaj.dubey 2017-04-07 8:06 ` pankaj.dubey [not found] ` <CGME20170330131421epcas5p40b0ad9c1003d3ab807667ae2b05d25bc@epcas5p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 02/12] ARM: EXYNOS: remove usage of soc_is_exynosMMMM from pm.c Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey 2017-04-07 8:24 ` Krzysztof Kozlowski 2017-04-07 8:24 ` Krzysztof Kozlowski 2017-04-07 12:12 ` pankaj.dubey 2017-04-07 12:12 ` pankaj.dubey 2017-04-07 12:24 ` Krzysztof Kozlowski 2017-04-07 12:24 ` Krzysztof Kozlowski 2017-04-07 14:13 ` pankaj.dubey 2017-04-07 14:13 ` pankaj.dubey [not found] ` <CGME20170330131424epcas5p4fccfab06e5d77d70d09c3835c3332ee5@epcas5p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 03/12] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey 2017-04-07 8:31 ` Krzysztof Kozlowski 2017-04-07 8:31 ` Krzysztof Kozlowski 2017-04-07 12:15 ` pankaj.dubey 2017-04-07 12:15 ` pankaj.dubey [not found] ` <CGME20170330131427epcas5p4c3d238022dc75694ad3baa8a1018ea04@epcas5p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 04/12] soc: samsung: add exynos chipid driver support Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey 2017-03-30 13:50 ` Arnd Bergmann 2017-03-30 13:50 ` Arnd Bergmann 2017-03-31 5:36 ` pankaj.dubey 2017-03-31 5:36 ` pankaj.dubey 2017-03-31 8:09 ` Arnd Bergmann 2017-03-31 8:09 ` Arnd Bergmann 2017-04-03 9:21 ` pankaj.dubey 2017-04-03 9:21 ` pankaj.dubey 2017-04-03 7:57 ` Marek Szyprowski 2017-04-03 7:57 ` Marek Szyprowski 2017-04-03 9:35 ` pankaj.dubey 2017-04-03 9:35 ` pankaj.dubey 2017-04-07 9:13 ` Krzysztof Kozlowski 2017-04-07 9:13 ` Krzysztof Kozlowski 2017-04-07 12:18 ` pankaj.dubey 2017-04-07 12:18 ` pankaj.dubey [not found] ` <CGME20170330131429epcas5p414565c5a9f2c38f3f6660b72f9ad68a2@epcas5p4.samsung.com> 2017-03-30 13:16 ` [PATCH v9 05/12] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS Pankaj Dubey 2017-03-30 13:16 ` Pankaj Dubey [not found] ` <CGME20170330131431epcas5p447a730e1bb194162f262a819ae665efb@epcas5p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 06/12] ARM64: " Pankaj Dubey 2017-03-30 13:17 ` Pankaj Dubey 2017-03-30 13:51 ` Arnd Bergmann 2017-03-30 13:51 ` Arnd Bergmann [not found] ` <CGME20170330131434epcas1p4e42fe06bae3456c39e0f93a2f8ae4bc0@epcas1p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 07/12] ARM: EXYNOS: introduce soc specific pm ops Pankaj Dubey 2017-03-30 13:17 ` Pankaj Dubey 2017-03-30 14:03 ` Arnd Bergmann 2017-03-30 14:03 ` Arnd Bergmann 2017-04-07 9:24 ` Krzysztof Kozlowski 2017-04-07 9:24 ` Krzysztof Kozlowski 2017-04-07 14:11 ` pankaj.dubey 2017-04-07 14:11 ` pankaj.dubey 2017-04-07 14:57 ` Krzysztof Kozlowski 2017-04-07 14:57 ` Krzysztof Kozlowski [not found] ` <CGME20170330131436epcas1p4a4f8bf7b64b4a5ff9ee692adc4e7d001@epcas1p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr,flag} ops to exynos_s2r_data Pankaj Dubey 2017-03-30 13:17 ` [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr, flag} " Pankaj Dubey 2017-04-07 10:32 ` Krzysztof Kozlowski [this message] 2017-04-07 10:32 ` Krzysztof Kozlowski 2017-04-07 13:52 ` [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr,flag} " pankaj.dubey 2017-04-07 13:52 ` [PATCH v9 08/12] ARM: EXYNOS: move exynos_boot_vector_{addr, flag} " pankaj.dubey [not found] ` <CGME20170330131438epcas1p459ce93da17fcd05249eddaef18d5021e@epcas1p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 09/12] ARM: EXYNOS: introduce exynos_cpu_info struct Pankaj Dubey 2017-03-30 13:17 ` Pankaj Dubey 2017-04-03 7:57 ` Marek Szyprowski 2017-04-03 7:57 ` Marek Szyprowski 2017-04-03 9:54 ` pankaj.dubey 2017-04-03 9:54 ` pankaj.dubey 2017-04-07 10:41 ` Krzysztof Kozlowski 2017-04-07 10:41 ` Krzysztof Kozlowski 2017-04-07 14:00 ` pankaj.dubey 2017-04-07 14:00 ` pankaj.dubey 2017-04-07 14:18 ` Krzysztof Kozlowski 2017-04-07 14:18 ` Krzysztof Kozlowski 2017-04-07 10:47 ` Krzysztof Kozlowski 2017-04-07 10:47 ` Krzysztof Kozlowski [not found] ` <CGME20170330131440epcas1p4f27192272761aa593b6cf083453e8adc@epcas1p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 10/12] ARM: EXYNOS: move power_{down,up} to per SoC struct exynos_cpu_info Pankaj Dubey 2017-03-30 13:17 ` [PATCH v9 10/12] ARM: EXYNOS: move power_{down, up} " Pankaj Dubey 2017-04-07 12:04 ` [PATCH v9 10/12] ARM: EXYNOS: move power_{down,up} " Krzysztof Kozlowski 2017-04-07 12:04 ` Krzysztof Kozlowski 2017-04-07 14:12 ` pankaj.dubey 2017-04-07 14:12 ` pankaj.dubey [not found] ` <CGME20170330131442epcas5p4f2be72cb3ec506847d8e832675e682c4@epcas5p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 11/12] ARM: EXYNOS: move cpu_restart as a SoC specific hook to exynos_cpu_info Pankaj Dubey 2017-03-30 13:17 ` Pankaj Dubey 2017-04-07 12:23 ` Krzysztof Kozlowski 2017-04-07 12:23 ` Krzysztof Kozlowski [not found] ` <CGME20170330131444epcas5p45051cf7e6ec7a993c2c9f84254b89a19@epcas5p4.samsung.com> 2017-03-30 13:17 ` [PATCH v9 12/12] ARM: EXYNOS: refactor of mach-exynos to use chipid information Pankaj Dubey 2017-03-30 13:17 ` Pankaj Dubey 2017-03-30 14:01 ` Arnd Bergmann 2017-03-30 14:01 ` Arnd Bergmann 2017-03-31 6:01 ` pankaj.dubey 2017-03-31 6:01 ` pankaj.dubey 2017-03-31 8:22 ` Arnd Bergmann 2017-03-31 8:22 ` Arnd Bergmann 2017-04-03 9:25 ` pankaj.dubey 2017-04-03 9:25 ` pankaj.dubey
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=CAJKOXPeFhmcscCX-so5qSx9pdX8oEmC+-283HO+J3vJNvWr_Hg@mail.gmail.com \ --to=krzk@kernel.org \ --cc=a.hajda@samsung.com \ --cc=arnd@arndb.de \ --cc=cwchoi00@gmail.com \ --cc=javier@osg.samsung.com \ --cc=kgene@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=m.reichl@fivetechno.de \ --cc=m.szyprowski@samsung.com \ --cc=pankaj.dubey@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.