lkml.org 
[lkml]   [2019]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
On Sun, Dec 08, 2019 at 07:24:49AM +0900, Hyunki Koo wrote:
>
> On 19. 12. 7. 오후 10:37, Krzysztof Kozlowski wrote:
> > On Sat, 7 Dec 2019 at 14:28, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > On Sat, Dec 07, 2019 at 10:00:48PM +0900, Hyunki Koo wrote:
> > > > From: Hyunki Koo <hyunki00.koo@samsung.com>
> > > >
> > > > Not all exynos device have IRQ_COMBINER.
> > > > Thus add the config for EXYNOS_IRQ_COMBINER.
> > > >
> > > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> > > > ---
> > > > drivers/irqchip/Kconfig | 7 +++++++
> > > > drivers/irqchip/Makefile | 2 +-
> > > > 2 files changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > Hi,
> > >
> > > There is no changelog and versioning of this patch so I do not
> > > understand how it differs with previous. It's a resend? v2? It brings
> > > the confusion and looks like you're ignoring previous comments.
> > >
> > > Looks the same and looks like breaking Exynos platforms in the same way.
> > >
> > > If you not want to skip combiner on ARMv8, it makes sense, then please
> > > follow the approach we did for Pinctrl drivers (PINCTRL_EXYNOS_ARM and
> > > PINCTRL_EXYNOS_ARM64).
> > >
> > > Best regards,
> > > Krzysztof
> > Ah, now I see the second patch. Still you break bisect which requires
> > specific ordering of patches or squashing them into one. Optionally
> > this could be default=y if ARCH_EXYNOS && ARM. I prefer just squashing
> > both into one patch in this case.
>
> you mean squashing two files arch/arm/mach-exynos/Kconfig and
> arch/arm/mach-exynos/Kconfig into one patch
>
> or squashing into only one file like blow?
>
> +config EXYNOS_IRQ_COMBINER
> +       bool "Samsung Exynos IRQ combiner support"
> +       depends on (ARCH_EXYNOS  && ARM) || COMPILE_TEST
> +       default y
>
> I prefer first one (squashing two files into one patch)

Squashing two patches into one.

>
> >
> > > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > > index ba152954324b..3ed7b7f2ae26 100644
> > > > --- a/drivers/irqchip/Kconfig
> > > > +++ b/drivers/irqchip/Kconfig
> > > > @@ -499,4 +499,11 @@ config SIFIVE_PLIC
> > > >
> > > > If you don't know what to do here, say Y.
> > > >
> > > > +config EXYNOS_IRQ_COMBINER
> > > > + bool "Samsung Exynos IRQ combiner support"
> > Now point it to be visible. Only for COMPILE_TEST

Typo from my side. I wanted to say "No point" - this should not be
selectable by user.

> >
> > > > + depends on ARCH_EXYNOS
> > Since you make it a separate option, make it COMPILE_TEST.
>
> Is this  good ?

Not entirely. The bool should be also with "if COMPILE TEST" so:

config EXYNOS_IRQ_COMBINER
bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST
depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST


Best regards,
Krzysztof

\
 
 \ /
  Last update: 2019-12-09 09:41    [W:0.034 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site