All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 6/6] arm64: enable RapidIO menu in Kconfig
Date: Wed, 1 Aug 2018 10:10:27 +0100	[thread overview]
Message-ID: <20180801091027.GC14438@arm.com> (raw)
In-Reply-To: <f2723578-f39d-27ac-b376-0f06905caa67@gmail.com>

On Tue, Jul 31, 2018 at 04:29:56PM -0400, Alex Bounine wrote:
> On 2018-07-31 08:54 AM, Alex Bounine wrote:
> >On 2018-07-31 04:41 AM, Will Deacon wrote:
> >>On Mon, Jul 30, 2018 at 06:50:34PM -0400, Alexei Colin wrote:
> >>>Platforms with a PCI bus will be offered the RapidIO menu since they may
> >>>be want support for a RapidIO PCI device. Platforms without a PCI bus
> >>>that might include a RapidIO IP block will need to "select HAS_RAPIDIO"
> >>>in the platform-/machine-specific "config ARCH_*" Kconfig entry.
> >>>
> >>>Tested that kernel builds for arm64 with RapidIO subsystem and
> >>>switch drivers enabled, also that the modules load successfully
> >>>on a custom Aarch64 Qemu model.
> >>>
> >>>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>Cc: Russell King <linux@armlinux.org.uk>
> >>>Cc: John Paul Walters <jwalters@isi.edu>
> >>>Cc: linux-arm-kernel at lists.infradead.org
> >>>Cc: linux-kernel at vger.kernel.org,
> >>>Signed-off-by: Alexei Colin <acolin@isi.edu>
> >>>---
> >>>? arch/arm64/Kconfig | 2 ++
> >>>? 1 file changed, 2 insertions(+)
> >>
> >>Thanks, this looks much cleaner than before:
> >>
> >>Acked-by: Will Deacon <will.deacon@arm.com>
> >>
> >>The only thing I'm not sure about is why we don't just select HAS_RAPIDIO
> >>unconditionally in the arm64 Kconfig. Does selecting only that option
> >>actually pull in new code to the build?
> >>
> >HAS_RAPIDIO option is intended for SOCs that have built in SRIO
> >controllers, like TI KeyStoneII or FPGAs. Because RapidIO subsystem core
> >is required during RapidIO port driver initialization, having separate
> >option allows us to control available build options for RapidIO core and
> >port driver (bool vs. tristate) and disable module option if port driver
> >is configured as built-in.
> 
> I am thinking about where HAS_RAPIDIO option can be set for arm64 branch.
> Having it set globally is too broad. For example we have Xilinx Zinq US
> board with SRIO IP on it. Having it globally in arm64 branch - bad. Probably
> having it set in drivers/soc/... is the best place.

Why is selecting HAS_RAPIDIO globally a bad thing to do? The way these
normally work is, if some subsystem requires arch support, then there's
an ARCH_HAS_xxxx option which the architecture selects when it implements
that support. Once you've enabled that, then that allows other sub-options
to be selected, such as specific drivers or what-not. Look at the Kconfig
files under drivers/soc/ -- you don't see anybody selecting ARCH_HAS_*
options.

Now, if HAS_RAPIDIO alone is pulling in a whole load of code to the build,
then it sounds like a misnomer.

Confused.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Alex Bounine <alex.bou9@gmail.com>
Cc: Alexei Colin <acolin@isi.edu>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Russell King <linux@armlinux.org.uk>,
	John Paul Walters <jwalters@isi.edu>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/6] arm64: enable RapidIO menu in Kconfig
Date: Wed, 1 Aug 2018 10:10:27 +0100	[thread overview]
Message-ID: <20180801091027.GC14438@arm.com> (raw)
In-Reply-To: <f2723578-f39d-27ac-b376-0f06905caa67@gmail.com>

On Tue, Jul 31, 2018 at 04:29:56PM -0400, Alex Bounine wrote:
> On 2018-07-31 08:54 AM, Alex Bounine wrote:
> >On 2018-07-31 04:41 AM, Will Deacon wrote:
> >>On Mon, Jul 30, 2018 at 06:50:34PM -0400, Alexei Colin wrote:
> >>>Platforms with a PCI bus will be offered the RapidIO menu since they may
> >>>be want support for a RapidIO PCI device. Platforms without a PCI bus
> >>>that might include a RapidIO IP block will need to "select HAS_RAPIDIO"
> >>>in the platform-/machine-specific "config ARCH_*" Kconfig entry.
> >>>
> >>>Tested that kernel builds for arm64 with RapidIO subsystem and
> >>>switch drivers enabled, also that the modules load successfully
> >>>on a custom Aarch64 Qemu model.
> >>>
> >>>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>Cc: Russell King <linux@armlinux.org.uk>
> >>>Cc: John Paul Walters <jwalters@isi.edu>
> >>>Cc: linux-arm-kernel@lists.infradead.org
> >>>Cc: linux-kernel@vger.kernel.org,
> >>>Signed-off-by: Alexei Colin <acolin@isi.edu>
> >>>---
> >>>  arch/arm64/Kconfig | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>
> >>Thanks, this looks much cleaner than before:
> >>
> >>Acked-by: Will Deacon <will.deacon@arm.com>
> >>
> >>The only thing I'm not sure about is why we don't just select HAS_RAPIDIO
> >>unconditionally in the arm64 Kconfig. Does selecting only that option
> >>actually pull in new code to the build?
> >>
> >HAS_RAPIDIO option is intended for SOCs that have built in SRIO
> >controllers, like TI KeyStoneII or FPGAs. Because RapidIO subsystem core
> >is required during RapidIO port driver initialization, having separate
> >option allows us to control available build options for RapidIO core and
> >port driver (bool vs. tristate) and disable module option if port driver
> >is configured as built-in.
> 
> I am thinking about where HAS_RAPIDIO option can be set for arm64 branch.
> Having it set globally is too broad. For example we have Xilinx Zinq US
> board with SRIO IP on it. Having it globally in arm64 branch - bad. Probably
> having it set in drivers/soc/... is the best place.

Why is selecting HAS_RAPIDIO globally a bad thing to do? The way these
normally work is, if some subsystem requires arch support, then there's
an ARCH_HAS_xxxx option which the architecture selects when it implements
that support. Once you've enabled that, then that allows other sub-options
to be selected, such as specific drivers or what-not. Look at the Kconfig
files under drivers/soc/ -- you don't see anybody selecting ARCH_HAS_*
options.

Now, if HAS_RAPIDIO alone is pulling in a whole load of code to the build,
then it sounds like a misnomer.

Confused.

Will

  parent reply	other threads:[~2018-08-01  9:10 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-30 22:50 [PATCH 0/6] rapidio: move Kconfig menu definition to subsystem Alexei Colin
2018-07-30 22:50 ` Alexei Colin
2018-07-30 22:50 ` [PATCH 1/6] rapidio: define top Kconfig menu in driver subtree Alexei Colin
2018-07-30 22:50 ` [PATCH 2/6] x86: factor out RapidIO Kconfig menu Alexei Colin
2018-07-30 22:55   ` [PATCH 2/6] x86: factor out RapidIO Kconfig menu; Thomas Gleixner
2018-07-31 14:30     ` Alex Bounine
2018-07-30 22:50 ` [PATCH 3/6] powerpc: factor out RapidIO Kconfig menu entry Alexei Colin
2018-07-31  9:45   ` Michael Ellerman
2018-07-30 22:50 ` [PATCH 4/6] mips: factor out RapidIO Kconfig entry Alexei Colin
2018-07-31  8:13   ` Alexander Sverdlin
2018-07-31  8:13     ` Alexander Sverdlin
2018-07-30 22:50 ` [PATCH 5/6] arm: enable RapidIO menu in Kconfig Alexei Colin
2018-07-30 22:50   ` Alexei Colin
2018-07-31 12:04   ` Christoph Hellwig
2018-07-31 12:04     ` Christoph Hellwig
2018-07-31 12:43     ` Alex Bounine
2018-07-31 12:43       ` Alex Bounine
2018-07-31 12:48       ` Russell King - ARM Linux
2018-07-31 12:48         ` Russell King - ARM Linux
2018-07-31 13:15         ` Alex Bounine
2018-07-31 13:15           ` Alex Bounine
2018-07-30 22:50 ` [PATCH 6/6] arm64: " Alexei Colin
2018-07-30 22:50   ` Alexei Colin
2018-07-31  8:41   ` Will Deacon
2018-07-31  8:41     ` Will Deacon
2018-07-31 12:54     ` Alex Bounine
2018-07-31 12:54       ` Alex Bounine
2018-07-31 15:52       ` Russell King - ARM Linux
2018-07-31 15:52         ` Russell King - ARM Linux
2018-07-31 17:59         ` Alex Bounine
2018-07-31 17:59           ` Alex Bounine
2018-07-31 18:18           ` Russell King - ARM Linux
2018-07-31 18:18             ` Russell King - ARM Linux
2018-07-31 20:01             ` Alex Bounine
2018-07-31 20:01               ` Alex Bounine
2018-08-01 10:38               ` Russell King - ARM Linux
2018-08-01 10:38                 ` Russell King - ARM Linux
2018-08-01 13:54                 ` Alexei Colin
2018-08-01 13:54                   ` Alexei Colin
2018-08-01 15:14                 ` Alex Bounine
2018-08-01 15:14                   ` Alex Bounine
2018-07-31 20:29       ` Alex Bounine
2018-07-31 20:29         ` Alex Bounine
2018-07-31 20:46         ` Alexei Colin
2018-07-31 20:46           ` Alexei Colin
2018-08-01 13:57           ` Alex Bounine
2018-08-01 13:57             ` Alex Bounine
2018-08-01  9:10         ` Will Deacon [this message]
2018-08-01  9:10           ` Will Deacon
2018-07-30 22:59 ` [PATCH 0/6] rapidio: move Kconfig menu definition to subsystem Russell King - ARM Linux
2018-07-30 22:59   ` Russell King - ARM Linux
2018-07-31  1:08 ` Randy Dunlap
2018-07-31  1:08   ` Randy Dunlap
2018-07-31 14:26 ` Alex Bounine
2018-07-31 14:26   ` Alex Bounine

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=20180801091027.GC14438@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 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.