Messages in this thread | | | Subject | Re: [RESEND PATCH 0/6] rapidio: move Kconfig menu definition to subsystem | From | Alex Bounine <> | Date | Tue, 31 Jul 2018 14:21:57 -0400 |
| |
On 2018-07-31 11:59 AM, Russell King - ARM Linux wrote: > For the thread associated with this patch set, a review of a previous > patch for ARM posted last Tuesday on this subject asked a series of > questions about the PCI-nature of this. The review has not been > responded to.
We are dealing with this now. More appropriate to do it this time than before having reworked set.
> If it is inappropriate to offer RapidIO for any architecture that > happens to has PCI, then it is inappropriate to offer it for any > ARM machine that happens to have PCI.
It is completely appropriate to use RapidIO on any architecture that has PCI/PCIe using existing PCIe-to-SRIO host bridges. Works well with Marvell and NVIDIA boards.
Confusion here is caused by the fact that there are ARM and non-ARM devices that offer on-chip RapidIO host controllers as well as PCIe. E.g. TI Keystone/KeystoneII, FSL 85xx/86xx, Xilinx and Altera FPGAs with ARM cores, Cavium on MIPS. In most cases external buses are configurable and we have to address possible combinations.
I already posted some explanation in response to your earlier comment.
> In light of the lack of explanation on this point so far, I'm naking > the ARM part of this series for now. >
Explanations posted.
> I also think that the HAS_RAPIDIO thing is misleading and needs > sorting out (as I've mentioned in other emails, including the one > I refer to above) before rapidio becomes available more widely. > Highly likely it is used right now in a base station of mobile operator near you :)
> On Tue, Jul 31, 2018 at 10:29:48AM -0400, Alexei Colin wrote: >> Resending the patchset from prior submission: >> https://lkml.org/lkml/2018/7/30/911 >> >> The only change are the Cc tags in all patches now include the mailing >> lists for all affected architectures, and patch 1/6 (which adds the menu >> item to RapdidIO subsystem Kconfig) is CCed to all maintainers who are >> getting this cover letter. The cover letter has been updated with >> explanations to points raised in the feedback. >> >> >> >> The top-level Kconfig entry for RapidIO subsystem is currently >> duplicated in several architecture-specific Kconfig files. This set of >> patches does two things: >> >> 1. Move the Kconfig menu definition into the RapidIO subsystem and >> remove the duplicate definitions from arch Kconfig files. >> >> 2. Enable RapidIO Kconfig menu entry for arm and arm64 architectures, >> where it was not enabled before. I tested that subsystem and drivers >> build successfully for both architectures, and tested that the modules >> load on a custom arm64 Qemu model. >> >> For all architectures, RapidIO menu should be offered when either: >> (1) The platform has a PCI bus (which host a RapidIO module on the bus). >> (2) The platform has a RapidIO IP block (connected to a system bus, e.g. >> AXI on ARM). In this case, 'select HAS_RAPIDIO' should be added to the >> 'config ARCH_*' menu entry for the SoCs that offer the IP block. >> >> Prior to this patchset, different architectures used different criteria: >> * powerpc: (1) and (2) >> * mips: (1) and (2) after recent commit into next that added (2): >> https://www.linux-mips.org/archives/linux-mips/2018-07/msg00596.html >> fc5d988878942e9b42a4de5204bdd452f3f1ce47 >> 491ec1553e0075f345fbe476a93775eabcbc40b6 >> * x86: (1) >> * arm,arm64: none (RapidIO menus never offered) >> >> This set of architectures are the ones that implement support for >> RapidIO as system bus. On some platforms RapidIO can be the only system >> bus available replacing PCI/PCIe. As it is done now, RapidIO is >> configured in "Bus Options" (x86/PPC) or "Bus Support" (ARMs) sub-menu >> and from system configuration option it should be kept this way. >> Current location of RAPIDIO configuration option is familiar to users of >> PowerPC and x86 platforms, and is similarly available in some ARM >> manufacturers kernel code trees. (Alex Bounine) >> >> HAS_RAPIDIO is not enabled unconditionally, because 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. (Alex Bounine) >> >> Responses to feedback from prior submission (thanks for the reviews!): >> http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/593347.html >> http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/593349.html >> >> Changelog: >> * Moved Kconfig entry into RapidIO subsystem instead of duplicating >> >> In the current patchset, I took the approach of adding '|| PCI' to the >> depends in the subsystem. I did try the alterantive approach mentioned >> in the reviews for v1 of this patch, where the subsystem Kconfig does >> not add a '|| PCI' and each per-architecture Kconfig has to add a >> 'select HAS_RAPIDIO if PCI' and SoCs with IP blocks have to also add >> 'select HAS_RAPIDIO'. This works too but requires each architecture's >> Kconfig to add the line for RapidIO (whereas current approach does not >> require that involvement) and also may create a false impression that >> the dependency on PCI is strict. >> >> We appreciate the suggestion for also selecting the RapdiIO subsystem for >> compilation with COMPILE_TEST, but hope to address it in a separate >> patchset, localized to the subsystem, since it will need to change >> depends on all drivers, not just on the top level, and since this >> patch now spans multiple architectures. >> >> Alexei Colin (6): >> rapidio: define top Kconfig menu in driver subtree >> x86: factor out RapidIO Kconfig menu >> powerpc: factor out RapidIO Kconfig menu entry >> mips: factor out RapidIO Kconfig entry >> arm: enable RapidIO menu in Kconfig >> arm64: enable RapidIO menu in Kconfig >> >> arch/arm/Kconfig | 2 ++ >> arch/arm64/Kconfig | 2 ++ >> arch/mips/Kconfig | 11 ----------- >> arch/powerpc/Kconfig | 13 +------------ >> arch/x86/Kconfig | 8 -------- >> drivers/rapidio/Kconfig | 15 +++++++++++++++ >> 6 files changed, 20 insertions(+), 31 deletions(-) >> >> -- >> 2.18.0 >> >
| |