| Message ID | 20190617060957.16171-1-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | spi: Kconfig: spi-synquacer: Added ARM and ARM64 dependence | expand |
On Mon, Jun 17, 2019 at 03:09:57PM +0900, Masahisa Kojima wrote: > kbuild test reported that Alpha and some of the architectures > are missing readsx/writesx functions. > spi-synquacer driver is only targeted for arm and arm64 platforms. > With that, added ARM and ARM64 dependence in Kconfig. Are you sure it's those functions (which only appear to be defined on arc according to a quick grep) and are you sure that there's no other Kconfig symbol specifically for those being defined which can be used rather than depending on specific architectures? > This patch also specifies the default compile type as module. This should be a separate patch and we don't generally change the default unless there's a reason to do so.
On Mon, 17 Jun 2019 at 12:17, Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 17, 2019 at 03:09:57PM +0900, Masahisa Kojima wrote: > > kbuild test reported that Alpha and some of the architectures > > are missing readsx/writesx functions. > > spi-synquacer driver is only targeted for arm and arm64 platforms. > > With that, added ARM and ARM64 dependence in Kconfig. > > Are you sure it's those functions (which only appear to be defined on > arc according to a quick grep) and are you sure that there's no other > Kconfig symbol specifically for those being defined which can be used > rather than depending on specific architectures? > I'm not sure I see the point of this. Building this driver for alpha and parisc has no use in practice, and does not provide any additional build coverage given that the driver can be enabled on any ARMA/ARM64 build. > > This patch also specifies the default compile type as module. > > This should be a separate patch and we don't generally change the > default unless there's a reason to do so.' That was my suggestion - just 'default m' is generally not accepted, but 'default m' for a driver if you enabled the ARCH_xxxx in question is reasonable, no?
On Mon, Jun 17, 2019 at 12:21:47PM +0200, Ard Biesheuvel wrote: > On Mon, 17 Jun 2019 at 12:17, Mark Brown <broonie@kernel.org> wrote: > > Are you sure it's those functions (which only appear to be defined on > > arc according to a quick grep) and are you sure that there's no other > > Kconfig symbol specifically for those being defined which can be used > > rather than depending on specific architectures? > I'm not sure I see the point of this. Building this driver for alpha > and parisc has no use in practice, and does not provide any additional > build coverage given that the driver can be enabled on any ARMA/ARM64 > build. It's helpful for compile test coverage, which in turn is useful for people doing subsystem or kernel wide changes. A dependency on ARM64 || COMPILE_TEST would make sense but wouldn't stop things getting built on the older archtiectures so you'd still want some dependency for that. > > > This patch also specifies the default compile type as module. > > This should be a separate patch and we don't generally change the > > default unless there's a reason to do so.' > That was my suggestion - just 'default m' is generally not accepted, > but 'default m' for a driver if you enabled the ARCH_xxxx in question > is reasonable, no? No, there's no special treatment for arch specific drivers here.
Hi Mark, There was a patch to address same compile error. https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1982194.html Does it preferable solution to use iowrite32_rep() series? On Mon, 17 Jun 2019 at 20:46, Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 17, 2019 at 12:21:47PM +0200, Ard Biesheuvel wrote: > > On Mon, 17 Jun 2019 at 12:17, Mark Brown <broonie@kernel.org> wrote: > > > > Are you sure it's those functions (which only appear to be defined on > > > arc according to a quick grep) and are you sure that there's no other > > > Kconfig symbol specifically for those being defined which can be used > > > rather than depending on specific architectures? > > > I'm not sure I see the point of this. Building this driver for alpha > > and parisc has no use in practice, and does not provide any additional > > build coverage given that the driver can be enabled on any ARMA/ARM64 > > build. > > It's helpful for compile test coverage, which in turn is useful for > people doing subsystem or kernel wide changes. A dependency on ARM64 || > COMPILE_TEST would make sense but wouldn't stop things getting built on > the older archtiectures so you'd still want some dependency for that. > > > > > This patch also specifies the default compile type as module. > > > > This should be a separate patch and we don't generally change the > > > default unless there's a reason to do so.' > > > That was my suggestion - just 'default m' is generally not accepted, > > but 'default m' for a driver if you enabled the ARCH_xxxx in question > > is reasonable, no? > > No, there's no special treatment for arch specific drivers here.
On Tue, Jun 18, 2019 at 05:13:57PM +0900, Masahisa Kojima wrote: Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed. > There was a patch to address same compile error. > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1982194.html > > Does it preferable solution to use iowrite32_rep() series? If there's a patch that removes the need to have a hard dependency on the particular architectures that'd definitely be better. Separately a patch adding a depends on the architectures with || COMPILE_TEST would be OK though and potentially useful like Ard says.
On Tue, 18 Jun 2019 at 19:57, Mark Brown <broonie@kernel.org> wrote: > > On Tue, Jun 18, 2019 at 05:13:57PM +0900, Masahisa Kojima wrote: > > Please don't top post, reply in line with needed context. This allows > readers to readily follow the flow of conversation and understand what > you are talking about and also helps ensure that everything in the > discussion is being addressed. I'm sorry, I understood. > > There was a patch to address same compile error. > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1982194.html > > > > Does it preferable solution to use iowrite32_rep() series? > > If there's a patch that removes the need to have a hard dependency on > the particular architectures that'd definitely be better. Separately a > patch adding a depends on the architectures with || COMPILE_TEST would > be OK though and potentially useful like Ard says. I will try to use iowrite32_rep() series.
On Tue, Jun 18, 2019 at 08:20:39PM +0900, Masahisa Kojima wrote: > On Tue, 18 Jun 2019 at 19:57, Mark Brown <broonie@kernel.org> wrote: > > On Tue, Jun 18, 2019 at 05:13:57PM +0900, Masahisa Kojima wrote: > > Please don't top post, reply in line with needed context. This allows > > readers to readily follow the flow of conversation and understand what > > you are talking about and also helps ensure that everything in the > > discussion is being addressed. > I'm sorry, I understood. Rather than writing your reply to a message at the top of the message as you did it's better to add your reply after some of the quoted message (as you've done here) since that makes it easier for people to read your message and follow what the reply is about.
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index ae9c81214298..667d341e8e95 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -736,6 +736,8 @@ config SPI_SUN6I config SPI_SYNQUACER tristate "Socionext's SynQuacer HighSpeed SPI controller" depends on ARCH_SYNQUACER || COMPILE_TEST + depends on ARM || ARM64 + default m if ARCH_SYNQUACER help SPI driver for Socionext's High speed SPI controller which provides various operating modes for interfacing to serial peripheral devices
kbuild test reported that Alpha and some of the architectures are missing readsx/writesx functions. spi-synquacer driver is only targeted for arm and arm64 platforms. With that, added ARM and ARM64 dependence in Kconfig. This patch also specifies the default compile type as module. Fixes: b0823ee35cf9b ("spi: Add spi driver for Socionext SynQuacer platform") Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- drivers/spi/Kconfig | 2 ++ 1 file changed, 2 insertions(+) -- 2.14.2