linux-samsung-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] remove direct dependancy between irq and exynos
@ 2019-12-07 13:00 Hyunki Koo
  2019-12-07 13:00 ` [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER Hyunki Koo
  2019-12-07 13:00 ` [PATCH 2/2] ARM: exynos: add select EXYNOS_IRQ_COMBINER Hyunki Koo
  0 siblings, 2 replies; 7+ messages in thread
From: Hyunki Koo @ 2019-12-07 13:00 UTC (permalink / raw)
  To: linux, kgene, krzk, tglx, jason, maz
  Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, kernel-team,
	Hyunki Koo

From: Hyunki Koo <hyunki00.koo@samsung.com>

This patch series is written to clean up dependancy of ARCH_EXYNOS
Not all exynos device have IRQ_COMBINER, especially aarch64 EXYNOS
but it is built for all exynos devices.
Thus add the config for EXYNOS_IRQ_COMBINER
remove direct between ARCH_EXYNOS and exynos-combiner.c
and only selected on the aarch32 devices

Hyunki Koo (2):
  irqchip: define EXYNOS_IRQ_COMBINER
  ARM: exynos: add select EXYNOS_IRQ_COMBINER

 arch/arm/mach-exynos/Kconfig | 1 +
 drivers/irqchip/Kconfig      | 7 +++++++
 drivers/irqchip/Makefile     | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
  2019-12-07 13:00 [PATCH 0/2] remove direct dependancy between irq and exynos Hyunki Koo
@ 2019-12-07 13:00 ` Hyunki Koo
  2019-12-07 13:28   ` Krzysztof Kozlowski
  2019-12-07 13:00 ` [PATCH 2/2] ARM: exynos: add select EXYNOS_IRQ_COMBINER Hyunki Koo
  1 sibling, 1 reply; 7+ messages in thread
From: Hyunki Koo @ 2019-12-07 13:00 UTC (permalink / raw)
  To: linux, kgene, krzk, tglx, jason, maz
  Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, kernel-team,
	Hyunki Koo

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(-)

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"
+	depends on ARCH_EXYNOS
+	help
+	  Say yes here to add support for the IRQ combiner devices embedded
+	  in Samsung Exynos chips.
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e806dda690ea..60d7c7260fc3 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
 obj-$(CONFIG_DAVINCI_AINTC)		+= irq-davinci-aintc.o
 obj-$(CONFIG_DAVINCI_CP_INTC)		+= irq-davinci-cp-intc.o
-obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
+obj-$(CONFIG_EXYNOS_IRQ_COMBINER)	+= exynos-combiner.o
 obj-$(CONFIG_FARADAY_FTINTC010)		+= irq-ftintc010.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_LPC32XX)		+= irq-lpc32xx.o
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] ARM: exynos: add select EXYNOS_IRQ_COMBINER
  2019-12-07 13:00 [PATCH 0/2] remove direct dependancy between irq and exynos Hyunki Koo
  2019-12-07 13:00 ` [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER Hyunki Koo
@ 2019-12-07 13:00 ` Hyunki Koo
  1 sibling, 0 replies; 7+ messages in thread
From: Hyunki Koo @ 2019-12-07 13:00 UTC (permalink / raw)
  To: linux, kgene, krzk, tglx, jason, maz
  Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, kernel-team,
	Hyunki Koo

From: Hyunki Koo <hyunki00.koo@samsung.com>

This patch add select EXYNOS_IRQ_COMBINER
on aarch32 Exynos devices

Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
---
 arch/arm/mach-exynos/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 4ef56571145b..c0efd544501e 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -18,6 +18,7 @@ menuconfig ARCH_EXYNOS
 	select EXYNOS_THERMAL
 	select EXYNOS_PMU
 	select EXYNOS_SROM
+	select EXYNOS_IRQ_COMBINER
 	select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS
 	select GPIOLIB
 	select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 && VIRTUALIZATION
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
  2019-12-07 13:00 ` [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER Hyunki Koo
@ 2019-12-07 13:28   ` Krzysztof Kozlowski
  2019-12-07 13:37     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2019-12-07 13:28 UTC (permalink / raw)
  To: Hyunki Koo
  Cc: linux, kgene, tglx, jason, maz, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, kernel-team, Hyunki Koo

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


> 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"
> +	depends on ARCH_EXYNOS
> +	help
> +	  Say yes here to add support for the IRQ combiner devices embedded
> +	  in Samsung Exynos chips.
> +
>  endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e806dda690ea..60d7c7260fc3 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
>  obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
>  obj-$(CONFIG_DAVINCI_AINTC)		+= irq-davinci-aintc.o
>  obj-$(CONFIG_DAVINCI_CP_INTC)		+= irq-davinci-cp-intc.o
> -obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
> +obj-$(CONFIG_EXYNOS_IRQ_COMBINER)	+= exynos-combiner.o
>  obj-$(CONFIG_FARADAY_FTINTC010)		+= irq-ftintc010.o
>  obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
>  obj-$(CONFIG_ARCH_LPC32XX)		+= irq-lpc32xx.o
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
  2019-12-07 13:28   ` Krzysztof Kozlowski
@ 2019-12-07 13:37     ` Krzysztof Kozlowski
  2019-12-07 22:24       ` Hyunki Koo
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2019-12-07 13:37 UTC (permalink / raw)
  To: Hyunki Koo
  Cc: linux, kgene, tglx, jason, maz, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, kernel-team, Hyunki Koo

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.

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

> > +     depends on ARCH_EXYNOS

Since you make it a separate option, make it COMPILE_TEST.

Best regards,
Krzysztof

> > +     help
> > +       Say yes here to add support for the IRQ combiner devices embedded
> > +       in Samsung Exynos chips.
> > +
> >  endmenu
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index e806dda690ea..60d7c7260fc3 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -9,7 +9,7 @@ obj-$(CONFIG_ARCH_BCM2835)            += irq-bcm2835.o
> >  obj-$(CONFIG_ARCH_BCM2835)           += irq-bcm2836.o
> >  obj-$(CONFIG_DAVINCI_AINTC)          += irq-davinci-aintc.o
> >  obj-$(CONFIG_DAVINCI_CP_INTC)                += irq-davinci-cp-intc.o
> > -obj-$(CONFIG_ARCH_EXYNOS)            += exynos-combiner.o
> > +obj-$(CONFIG_EXYNOS_IRQ_COMBINER)    += exynos-combiner.o
> >  obj-$(CONFIG_FARADAY_FTINTC010)              += irq-ftintc010.o
> >  obj-$(CONFIG_ARCH_HIP04)             += irq-hip04.o
> >  obj-$(CONFIG_ARCH_LPC32XX)           += irq-lpc32xx.o
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
  2019-12-07 13:37     ` Krzysztof Kozlowski
@ 2019-12-07 22:24       ` Hyunki Koo
  2019-12-09  8:41         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 7+ messages in thread
From: Hyunki Koo @ 2019-12-07 22:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux, kgene, tglx, jason, maz, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, kernel-team, Hyunki Koo


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)

>
>>> 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
>
>>> +     depends on ARCH_EXYNOS
> Since you make it a separate option, make it COMPILE_TEST.

Is this  good ?

+config EXYNOS_IRQ_COMBINER

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

>
> Best regards,
> Krzysztof
>
>>> +     help
>>> +       Say yes here to add support for the IRQ combiner devices embedded
>>> +       in Samsung Exynos chips.
>>> +
>>>   endmenu
>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>> index e806dda690ea..60d7c7260fc3 100644
>>> --- a/drivers/irqchip/Makefile
>>> +++ b/drivers/irqchip/Makefile
>>> @@ -9,7 +9,7 @@ obj-$(CONFIG_ARCH_BCM2835)            += irq-bcm2835.o
>>>   obj-$(CONFIG_ARCH_BCM2835)           += irq-bcm2836.o
>>>   obj-$(CONFIG_DAVINCI_AINTC)          += irq-davinci-aintc.o
>>>   obj-$(CONFIG_DAVINCI_CP_INTC)                += irq-davinci-cp-intc.o
>>> -obj-$(CONFIG_ARCH_EXYNOS)            += exynos-combiner.o
>>> +obj-$(CONFIG_EXYNOS_IRQ_COMBINER)    += exynos-combiner.o
>>>   obj-$(CONFIG_FARADAY_FTINTC010)              += irq-ftintc010.o
>>>   obj-$(CONFIG_ARCH_HIP04)             += irq-hip04.o
>>>   obj-$(CONFIG_ARCH_LPC32XX)           += irq-lpc32xx.o
>>> --
>>> 2.17.1
>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER
  2019-12-07 22:24       ` Hyunki Koo
@ 2019-12-09  8:41         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2019-12-09  8:41 UTC (permalink / raw)
  To: Hyunki Koo
  Cc: linux, kgene, tglx, jason, maz, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, kernel-team, Hyunki Koo

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-12-09  8:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-07 13:00 [PATCH 0/2] remove direct dependancy between irq and exynos Hyunki Koo
2019-12-07 13:00 ` [PATCH 1/2] irqchip: define EXYNOS_IRQ_COMBINER Hyunki Koo
2019-12-07 13:28   ` Krzysztof Kozlowski
2019-12-07 13:37     ` Krzysztof Kozlowski
2019-12-07 22:24       ` Hyunki Koo
2019-12-09  8:41         ` Krzysztof Kozlowski
2019-12-07 13:00 ` [PATCH 2/2] ARM: exynos: add select EXYNOS_IRQ_COMBINER Hyunki Koo

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).