* [PATCH] ptp: ocp: don't allow on S390 @ 2021-08-13 20:30 Randy Dunlap 2021-08-16 10:20 ` patchwork-bot+netdevbpf ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Randy Dunlap @ 2021-08-13 20:30 UTC (permalink / raw) To: netdev; +Cc: Randy Dunlap, Richard Cochran, Jonathan Lemon Fix kconfig warning on arch/s390/: WARNING: unmet direct dependencies detected for SERIAL_8250 Depends on [n]: TTY [=y] && HAS_IOMEM [=y] && !S390 [=y] Selected by [m]: - PTP_1588_CLOCK_OCP [=m] && PTP_1588_CLOCK [=m] && HAS_IOMEM [=y] && PCI [=y] && SPI [=y] && I2C [=m] && MTD [=m] Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Richard Cochran <richardcochran@gmail.com> Cc: Jonathan Lemon <jonathan.lemon@gmail.com> --- There is no 8250 serial on S390. See commit 1598e38c0770. Is this driver useful even without 8250 serial? drivers/ptp/Kconfig | 1 + 1 file changed, 1 insertion(+) --- linux-next-20210813.orig/drivers/ptp/Kconfig +++ linux-next-20210813/drivers/ptp/Kconfig @@ -158,6 +158,7 @@ config PTP_1588_CLOCK_OCP depends on PTP_1588_CLOCK depends on HAS_IOMEM && PCI depends on SPI && I2C && MTD + depends on !S390 imply SPI_MEM imply SPI_XILINX imply MTD_SPI_NOR ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-13 20:30 [PATCH] ptp: ocp: don't allow on S390 Randy Dunlap @ 2021-08-16 10:20 ` patchwork-bot+netdevbpf 2021-08-16 21:09 ` Jonathan Lemon 2021-08-20 10:45 ` Arnd Bergmann 2 siblings, 0 replies; 19+ messages in thread From: patchwork-bot+netdevbpf @ 2021-08-16 10:20 UTC (permalink / raw) To: Randy Dunlap; +Cc: netdev, richardcochran, jonathan.lemon Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Fri, 13 Aug 2021 13:30:26 -0700 you wrote: > Fix kconfig warning on arch/s390/: > > WARNING: unmet direct dependencies detected for SERIAL_8250 > Depends on [n]: TTY [=y] && HAS_IOMEM [=y] && !S390 [=y] > Selected by [m]: > - PTP_1588_CLOCK_OCP [=m] && PTP_1588_CLOCK [=m] && HAS_IOMEM [=y] && PCI [=y] && SPI [=y] && I2C [=m] && MTD [=m] > > [...] Here is the summary with links: - ptp: ocp: don't allow on S390 https://git.kernel.org/netdev/net-next/c/944f510176eb You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-13 20:30 [PATCH] ptp: ocp: don't allow on S390 Randy Dunlap 2021-08-16 10:20 ` patchwork-bot+netdevbpf @ 2021-08-16 21:09 ` Jonathan Lemon 2021-08-16 21:15 ` Randy Dunlap 2021-08-20 10:45 ` Arnd Bergmann 2 siblings, 1 reply; 19+ messages in thread From: Jonathan Lemon @ 2021-08-16 21:09 UTC (permalink / raw) To: Randy Dunlap; +Cc: netdev, Richard Cochran On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: > There is no 8250 serial on S390. See commit 1598e38c0770. There's a 8250 serial device on the PCI card. Its been ages since I've worked on the architecture, but does S390 even support PCI? > Is this driver useful even without 8250 serial? The FB timecard has an FPGA that will internally parse the GNSS strings and correct the clock, so the PTP clock will work even without the serial devices. However, there are userspace tools which want to read the GNSS signal (for holdolver and leap second indication), which is why they are exposed. -- Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-16 21:09 ` Jonathan Lemon @ 2021-08-16 21:15 ` Randy Dunlap 2021-08-16 21:41 ` Jonathan Lemon 0 siblings, 1 reply; 19+ messages in thread From: Randy Dunlap @ 2021-08-16 21:15 UTC (permalink / raw) To: Jonathan Lemon; +Cc: netdev, Richard Cochran On 8/16/21 2:09 PM, Jonathan Lemon wrote: > On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: >> There is no 8250 serial on S390. See commit 1598e38c0770. > > There's a 8250 serial device on the PCI card. Its been > ages since I've worked on the architecture, but does S390 > even support PCI? Yes, it does. >> Is this driver useful even without 8250 serial? > > The FB timecard has an FPGA that will internally parse the > GNSS strings and correct the clock, so the PTP clock will > work even without the serial devices. > > However, there are userspace tools which want to read the > GNSS signal (for holdolver and leap second indication), > which is why they are exposed. So what do you recommend here? thanks. -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-16 21:15 ` Randy Dunlap @ 2021-08-16 21:41 ` Jonathan Lemon 2021-08-19 22:58 ` Randy Dunlap 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Lemon @ 2021-08-16 21:41 UTC (permalink / raw) To: Randy Dunlap; +Cc: netdev, Richard Cochran On Mon, Aug 16, 2021 at 02:15:51PM -0700, Randy Dunlap wrote: > On 8/16/21 2:09 PM, Jonathan Lemon wrote: > > On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: > > > There is no 8250 serial on S390. See commit 1598e38c0770. > > > > There's a 8250 serial device on the PCI card. Its been > > ages since I've worked on the architecture, but does S390 > > even support PCI? > > Yes, it does. > > > > Is this driver useful even without 8250 serial? > > > > The FB timecard has an FPGA that will internally parse the > > GNSS strings and correct the clock, so the PTP clock will > > work even without the serial devices. > > > > However, there are userspace tools which want to read the > > GNSS signal (for holdolver and leap second indication), > > which is why they are exposed. > > So what do you recommend here? Looking at 1598e38c0770, it appears the 8250 console is the problem. Couldn't S390 be fenced by SERIAL_8250_CONSOLE, instead of SERIAL_8250, which would make the 8250 driver available? For now, just disabling the driver on S390 sounds reasonable. -- Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-16 21:41 ` Jonathan Lemon @ 2021-08-19 22:58 ` Randy Dunlap 2021-08-20 8:02 ` Christian Borntraeger 0 siblings, 1 reply; 19+ messages in thread From: Randy Dunlap @ 2021-08-19 22:58 UTC (permalink / raw) To: Jonathan Lemon, Christian Borntraeger, Hendrik Brueckner Cc: netdev, Richard Cochran, Greg KH On 8/16/21 2:41 PM, Jonathan Lemon wrote: > On Mon, Aug 16, 2021 at 02:15:51PM -0700, Randy Dunlap wrote: >> On 8/16/21 2:09 PM, Jonathan Lemon wrote: >>> On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: >>>> There is no 8250 serial on S390. See commit 1598e38c0770. >>> >>> There's a 8250 serial device on the PCI card. Its been >>> ages since I've worked on the architecture, but does S390 >>> even support PCI? >> >> Yes, it does. >> >>>> Is this driver useful even without 8250 serial? >>> >>> The FB timecard has an FPGA that will internally parse the >>> GNSS strings and correct the clock, so the PTP clock will >>> work even without the serial devices. >>> >>> However, there are userspace tools which want to read the >>> GNSS signal (for holdolver and leap second indication), >>> which is why they are exposed. >> >> So what do you recommend here? > > Looking at 1598e38c0770, it appears the 8250 console is the > problem. Couldn't S390 be fenced by SERIAL_8250_CONSOLE, instead > of SERIAL_8250, which would make the 8250 driver available? OK, that sounds somewhat reasonable. > For now, just disabling the driver on S390 sounds reasonable. > S390 people, how does this look to you? This still avoids having serial 8250 console conflicting with S390's sclp console. (reference commit 1598e38c0770) --- drivers/tty/serial/8250/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux-next-20210819.orig/drivers/tty/serial/8250/Kconfig +++ linux-next-20210819/drivers/tty/serial/8250/Kconfig @@ -6,7 +6,6 @@ config SERIAL_8250 tristate "8250/16550 and compatible serial support" - depends on !S390 select SERIAL_CORE select SERIAL_MCTRL_GPIO if GPIOLIB help @@ -85,6 +84,7 @@ config SERIAL_8250_FINTEK config SERIAL_8250_CONSOLE bool "Console on 8250/16550 and compatible serial port" depends on SERIAL_8250=y + depends on !S390 select SERIAL_CORE_CONSOLE select SERIAL_EARLYCON help -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-19 22:58 ` Randy Dunlap @ 2021-08-20 8:02 ` Christian Borntraeger 2021-08-20 9:59 ` Niklas Schnelle 0 siblings, 1 reply; 19+ messages in thread From: Christian Borntraeger @ 2021-08-20 8:02 UTC (permalink / raw) To: Randy Dunlap, Jonathan Lemon, Hendrik Brueckner, linux-s390 Cc: netdev, Richard Cochran, Greg KH On 20.08.21 00:58, Randy Dunlap wrote: > On 8/16/21 2:41 PM, Jonathan Lemon wrote: >> On Mon, Aug 16, 2021 at 02:15:51PM -0700, Randy Dunlap wrote: >>> On 8/16/21 2:09 PM, Jonathan Lemon wrote: >>>> On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: >>>>> There is no 8250 serial on S390. See commit 1598e38c0770. >>>> >>>> There's a 8250 serial device on the PCI card. Its been >>>> ages since I've worked on the architecture, but does S390 >>>> even support PCI? >>> >>> Yes, it does. We do support PCI, but only a (very) limited amount of cards. So there never will be a PCI card with 8250 on s390 and I also doubt that we will see the "OpenCompute TimeCard" on s390. So in essence the original patch is ok but the patch below would also be ok for KVM. But it results in a larger kernel with code that will never be used. So I guess the original patch is the better choice. >>> >>>>> Is this driver useful even without 8250 serial? >>>> >>>> The FB timecard has an FPGA that will internally parse the >>>> GNSS strings and correct the clock, so the PTP clock will >>>> work even without the serial devices. >>>> >>>> However, there are userspace tools which want to read the >>>> GNSS signal (for holdolver and leap second indication), >>>> which is why they are exposed. >>> >>> So what do you recommend here? >> >> Looking at 1598e38c0770, it appears the 8250 console is the >> problem. Couldn't S390 be fenced by SERIAL_8250_CONSOLE, instead >> of SERIAL_8250, which would make the 8250 driver available? > > OK, that sounds somewhat reasonable. > >> For now, just disabling the driver on S390 sounds reasonable. >> > > S390 people, how does this look to you? > > This still avoids having serial 8250 console conflicting > with S390's sclp console. > (reference commit 1598e38c0770) > > > --- > drivers/tty/serial/8250/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- linux-next-20210819.orig/drivers/tty/serial/8250/Kconfig > +++ linux-next-20210819/drivers/tty/serial/8250/Kconfig > @@ -6,7 +6,6 @@ > > config SERIAL_8250 > tristate "8250/16550 and compatible serial support" > - depends on !S390 > select SERIAL_CORE > select SERIAL_MCTRL_GPIO if GPIOLIB > help > @@ -85,6 +84,7 @@ config SERIAL_8250_FINTEK > config SERIAL_8250_CONSOLE > bool "Console on 8250/16550 and compatible serial port" > depends on SERIAL_8250=y > + depends on !S390 > select SERIAL_CORE_CONSOLE > select SERIAL_EARLYCON > help > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-20 8:02 ` Christian Borntraeger @ 2021-08-20 9:59 ` Niklas Schnelle 0 siblings, 0 replies; 19+ messages in thread From: Niklas Schnelle @ 2021-08-20 9:59 UTC (permalink / raw) To: Christian Borntraeger, Randy Dunlap, Jonathan Lemon, Hendrik Brueckner, linux-s390 Cc: netdev, Richard Cochran, Greg KH On Fri, 2021-08-20 at 10:02 +0200, Christian Borntraeger wrote: > > On 20.08.21 00:58, Randy Dunlap wrote: > > On 8/16/21 2:41 PM, Jonathan Lemon wrote: > > > On Mon, Aug 16, 2021 at 02:15:51PM -0700, Randy Dunlap wrote: > > > > On 8/16/21 2:09 PM, Jonathan Lemon wrote: > > > > > On Fri, Aug 13, 2021 at 01:30:26PM -0700, Randy Dunlap wrote: > > > > > > There is no 8250 serial on S390. See commit 1598e38c0770. > > > > > > > > > > There's a 8250 serial device on the PCI card. Its been > > > > > ages since I've worked on the architecture, but does S390 > > > > > even support PCI? > > > > > > > > Yes, it does. > > We do support PCI, but only a (very) limited amount of cards. > So there never will be a PCI card with 8250 on s390 and > I also doubt that we will see the "OpenCompute TimeCard" > on s390. > > So in essence the original patch is ok but the patch below > would also be ok for KVM. But it results in a larger kernel > with code that will never be used. So I guess the original > patch is the better choice. It looks to me like the SERIAL_8250 driver can be build as a module so would then not increase the kernel image size or am I missing something? In that case I would vote for the patch below. For PCI on s390 we do intend to, in principle, support arbitrary PCI devices and have already seen cases where non-trivial cards that were never before tested on s390 did "just work" once someone needed them. While I do agree that both 8250 and the Time Card are unlikely to be used on s390 never say never and compile testing a variety of driver code against our PCI primitives is good for quality control. In the end I'm okay with either. > > > > > > > Is this driver useful even without 8250 serial? > > > > > > > > > > The FB timecard has an FPGA that will internally parse the > > > > > GNSS strings and correct the clock, so the PTP clock will > > > > > work even without the serial devices. > > > > > > > > > > However, there are userspace tools which want to read the > > > > > GNSS signal (for holdolver and leap second indication), > > > > > which is why they are exposed. > > > > > > > > So what do you recommend here? > > > > > > Looking at 1598e38c0770, it appears the 8250 console is the > > > problem. Couldn't S390 be fenced by SERIAL_8250_CONSOLE, instead > > > of SERIAL_8250, which would make the 8250 driver available? > > > > OK, that sounds somewhat reasonable. > > > > > For now, just disabling the driver on S390 sounds reasonable. > > > > > > > S390 people, how does this look to you? > > > > This still avoids having serial 8250 console conflicting > > with S390's sclp console. > > (reference commit 1598e38c0770) > > > > > > --- > > drivers/tty/serial/8250/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- linux-next-20210819.orig/drivers/tty/serial/8250/Kconfig > > +++ linux-next-20210819/drivers/tty/serial/8250/Kconfig > > @@ -6,7 +6,6 @@ > > > > config SERIAL_8250 > > tristate "8250/16550 and compatible serial support" > > - depends on !S390 > > select SERIAL_CORE > > select SERIAL_MCTRL_GPIO if GPIOLIB > > help > > @@ -85,6 +84,7 @@ config SERIAL_8250_FINTEK > > config SERIAL_8250_CONSOLE > > bool "Console on 8250/16550 and compatible serial port" > > depends on SERIAL_8250=y > > + depends on !S390 > > select SERIAL_CORE_CONSOLE > > select SERIAL_EARLYCON > > help > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-13 20:30 [PATCH] ptp: ocp: don't allow on S390 Randy Dunlap 2021-08-16 10:20 ` patchwork-bot+netdevbpf 2021-08-16 21:09 ` Jonathan Lemon @ 2021-08-20 10:45 ` Arnd Bergmann 2021-08-20 15:31 ` Richard Cochran 2 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2021-08-20 10:45 UTC (permalink / raw) To: Randy Dunlap; +Cc: Networking, Richard Cochran, Jonathan Lemon On Fri, Aug 13, 2021 at 10:30 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > Fix kconfig warning on arch/s390/: > > WARNING: unmet direct dependencies detected for SERIAL_8250 > Depends on [n]: TTY [=y] && HAS_IOMEM [=y] && !S390 [=y] > Selected by [m]: > - PTP_1588_CLOCK_OCP [=m] && PTP_1588_CLOCK [=m] && HAS_IOMEM [=y] && PCI [=y] && SPI [=y] && I2C [=m] && MTD [=m] > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Richard Cochran <richardcochran@gmail.com> > Cc: Jonathan Lemon <jonathan.lemon@gmail.com> > --- > There is no 8250 serial on S390. See commit 1598e38c0770. > Is this driver useful even without 8250 serial? I think an easier way to do this would be to remove the 'select SERIAL_8250', I don't think that is actually a compile-time dependency, just something that you normally want to enable to make the device useful. I would also suggest removing all the 'imply' statements, they usually don't do what the original author intended anyway. If there is a compile-time dependency with those drivers, it should be 'depends on', otherwise they can normally be left out. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-20 10:45 ` Arnd Bergmann @ 2021-08-20 15:31 ` Richard Cochran 2021-08-24 21:48 ` Randy Dunlap 0 siblings, 1 reply; 19+ messages in thread From: Richard Cochran @ 2021-08-20 15:31 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Randy Dunlap, Networking, Jonathan Lemon On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > I would also suggest removing all the 'imply' statements, they > usually don't do what the original author intended anyway. > If there is a compile-time dependency with those drivers, > it should be 'depends on', otherwise they can normally be > left out. +1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-20 15:31 ` Richard Cochran @ 2021-08-24 21:48 ` Randy Dunlap 2021-08-25 10:55 ` Arnd Bergmann 0 siblings, 1 reply; 19+ messages in thread From: Randy Dunlap @ 2021-08-24 21:48 UTC (permalink / raw) To: Richard Cochran, Arnd Bergmann; +Cc: Networking, Jonathan Lemon On 8/20/21 8:31 AM, Richard Cochran wrote: > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > >> I would also suggest removing all the 'imply' statements, they >> usually don't do what the original author intended anyway. >> If there is a compile-time dependency with those drivers, >> it should be 'depends on', otherwise they can normally be >> left out. > > +1 Hi, Removing the "imply" statements is simple enough and the driver still builds cleanly without them, so Yes, they aren't needed here. Removing the SPI dependency is also clean. The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they can't be removed without some other driver changes, like using #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-24 21:48 ` Randy Dunlap @ 2021-08-25 10:55 ` Arnd Bergmann 2021-08-25 17:08 ` Jonathan Lemon 0 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2021-08-25 10:55 UTC (permalink / raw) To: Randy Dunlap; +Cc: Richard Cochran, Networking, Jonathan Lemon On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 8/20/21 8:31 AM, Richard Cochran wrote: > > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > > > >> I would also suggest removing all the 'imply' statements, they > >> usually don't do what the original author intended anyway. > >> If there is a compile-time dependency with those drivers, > >> it should be 'depends on', otherwise they can normally be > >> left out. > > > > +1 > > Hi, > > Removing the "imply" statements is simple enough and the driver > still builds cleanly without them, so Yes, they aren't needed here. > > Removing the SPI dependency is also clean. > > The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they > can't be removed without some other driver changes, like using > #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. If the SERIAL_8250 dependency is actually required, then using 'depends on' for this is probably better than an IS_ENABLED() check. The 'select' is definitely misplaced here, that doesn't even work when the dependencies fo 8250 itself are not met, and it does force-enable the entire TTY subsystem. Arnd Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 10:55 ` Arnd Bergmann @ 2021-08-25 17:08 ` Jonathan Lemon 2021-08-25 17:29 ` Randy Dunlap 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Lemon @ 2021-08-25 17:08 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Randy Dunlap, Richard Cochran, Networking On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: > On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > On 8/20/21 8:31 AM, Richard Cochran wrote: > > > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > > > > > >> I would also suggest removing all the 'imply' statements, they > > >> usually don't do what the original author intended anyway. > > >> If there is a compile-time dependency with those drivers, > > >> it should be 'depends on', otherwise they can normally be > > >> left out. > > > > > > +1 > > > > Hi, > > > > Removing the "imply" statements is simple enough and the driver > > still builds cleanly without them, so Yes, they aren't needed here. > > > > Removing the SPI dependency is also clean. > > > > The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they > > can't be removed without some other driver changes, like using > > #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. > > If the SERIAL_8250 dependency is actually required, then using > 'depends on' for this is probably better than an IS_ENABLED() check. > The 'select' is definitely misplaced here, that doesn't even work when > the dependencies fo 8250 itself are not met, and it does force-enable > the entire TTY subsystem. So, something like the following (untested) patch? I admit to not fully understanding all the nuances around Kconfig. -- Jonathan diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 32660dc11354..c3372efd1bb7 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig @@ -171,15 +171,10 @@ config PTP_1588_CLOCK_OCP tristate "OpenCompute TimeCard as PTP clock" depends on PTP_1588_CLOCK depends on HAS_IOMEM && PCI - depends on SPI && I2C && MTD + depends on I2C && MTD + depends on SERIAL_8250 depends on !S390 - imply SPI_MEM - imply SPI_XILINX - imply MTD_SPI_NOR - imply I2C_XILINX - select SERIAL_8250 select NET_DEVLINK - default n help This driver adds support for an OpenCompute time card. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 17:08 ` Jonathan Lemon @ 2021-08-25 17:29 ` Randy Dunlap 2021-08-25 18:05 ` Arnd Bergmann 2021-08-25 20:40 ` Jonathan Lemon 0 siblings, 2 replies; 19+ messages in thread From: Randy Dunlap @ 2021-08-25 17:29 UTC (permalink / raw) To: Jonathan Lemon, Arnd Bergmann; +Cc: Richard Cochran, Networking On 8/25/21 10:08 AM, Jonathan Lemon wrote: > On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: >> On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>> >>> On 8/20/21 8:31 AM, Richard Cochran wrote: >>>> On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: >>>> >>>>> I would also suggest removing all the 'imply' statements, they >>>>> usually don't do what the original author intended anyway. >>>>> If there is a compile-time dependency with those drivers, >>>>> it should be 'depends on', otherwise they can normally be >>>>> left out. >>>> >>>> +1 >>> >>> Hi, >>> >>> Removing the "imply" statements is simple enough and the driver >>> still builds cleanly without them, so Yes, they aren't needed here. >>> >>> Removing the SPI dependency is also clean. >>> >>> The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they >>> can't be removed without some other driver changes, like using >>> #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. >> >> If the SERIAL_8250 dependency is actually required, then using >> 'depends on' for this is probably better than an IS_ENABLED() check. >> The 'select' is definitely misplaced here, that doesn't even work when >> the dependencies fo 8250 itself are not met, and it does force-enable >> the entire TTY subsystem. > > So, something like the following (untested) patch? > I admit to not fully understanding all the nuances around Kconfig. Hi, You can also remove the "select NET_DEVLINK". The driver builds fine without it. And please drop the "default n" while at it. After that, your patch will match my (tested) patch. :) thanks. -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 17:29 ` Randy Dunlap @ 2021-08-25 18:05 ` Arnd Bergmann 2021-08-25 20:40 ` Jonathan Lemon 1 sibling, 0 replies; 19+ messages in thread From: Arnd Bergmann @ 2021-08-25 18:05 UTC (permalink / raw) To: Randy Dunlap; +Cc: Jonathan Lemon, Richard Cochran, Networking On Wed, Aug 25, 2021 at 7:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > On 8/25/21 10:08 AM, Jonathan Lemon wrote: > > So, something like the following (untested) patch? > > I admit to not fully understanding all the nuances around Kconfig. > > You can also remove the "select NET_DEVLINK". The driver builds fine > without it. And please drop the "default n" while at it. > > After that, your patch will match my (tested) patch. :) That version sounds good to me. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 17:29 ` Randy Dunlap 2021-08-25 18:05 ` Arnd Bergmann @ 2021-08-25 20:40 ` Jonathan Lemon 2021-08-25 20:45 ` Randy Dunlap 1 sibling, 1 reply; 19+ messages in thread From: Jonathan Lemon @ 2021-08-25 20:40 UTC (permalink / raw) To: Randy Dunlap; +Cc: Arnd Bergmann, Richard Cochran, Networking On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote: > On 8/25/21 10:08 AM, Jonathan Lemon wrote: > > On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: > > > On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > > > On 8/20/21 8:31 AM, Richard Cochran wrote: > > > > > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > > > > > > > > > > > I would also suggest removing all the 'imply' statements, they > > > > > > usually don't do what the original author intended anyway. > > > > > > If there is a compile-time dependency with those drivers, > > > > > > it should be 'depends on', otherwise they can normally be > > > > > > left out. > > > > > > > > > > +1 > > > > > > > > Hi, > > > > > > > > Removing the "imply" statements is simple enough and the driver > > > > still builds cleanly without them, so Yes, they aren't needed here. > > > > > > > > Removing the SPI dependency is also clean. > > > > > > > > The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they > > > > can't be removed without some other driver changes, like using > > > > #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. > > > > > > If the SERIAL_8250 dependency is actually required, then using > > > 'depends on' for this is probably better than an IS_ENABLED() check. > > > The 'select' is definitely misplaced here, that doesn't even work when > > > the dependencies fo 8250 itself are not met, and it does force-enable > > > the entire TTY subsystem. > > > > So, something like the following (untested) patch? > > I admit to not fully understanding all the nuances around Kconfig. > > Hi, > > You can also remove the "select NET_DEVLINK". The driver builds fine > without it. And please drop the "default n" while at it. I had to add this one because devlink is a dependency and the kbuild robot generated a config without it. The 'imply' statements were added because while the driver builds without them, the resources don't show up unless the platform modules are also present. This was really confusing users, since they selected the OCP driver and then were not able to use the flash since the XILINX modules had not been selected. Is there a better way of specifying these type of dependencies? -- Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 20:40 ` Jonathan Lemon @ 2021-08-25 20:45 ` Randy Dunlap 2021-08-25 21:14 ` Jonathan Lemon 0 siblings, 1 reply; 19+ messages in thread From: Randy Dunlap @ 2021-08-25 20:45 UTC (permalink / raw) To: Jonathan Lemon; +Cc: Arnd Bergmann, Richard Cochran, Networking On 8/25/21 1:40 PM, Jonathan Lemon wrote: > On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote: >> On 8/25/21 10:08 AM, Jonathan Lemon wrote: >>> On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: >>>> On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>>>> >>>>> On 8/20/21 8:31 AM, Richard Cochran wrote: >>>>>> On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: >>>>>> >>>>>>> I would also suggest removing all the 'imply' statements, they >>>>>>> usually don't do what the original author intended anyway. >>>>>>> If there is a compile-time dependency with those drivers, >>>>>>> it should be 'depends on', otherwise they can normally be >>>>>>> left out. >>>>>> >>>>>> +1 >>>>> >>>>> Hi, >>>>> >>>>> Removing the "imply" statements is simple enough and the driver >>>>> still builds cleanly without them, so Yes, they aren't needed here. >>>>> >>>>> Removing the SPI dependency is also clean. >>>>> >>>>> The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they >>>>> can't be removed without some other driver changes, like using >>>>> #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. >>>> >>>> If the SERIAL_8250 dependency is actually required, then using >>>> 'depends on' for this is probably better than an IS_ENABLED() check. >>>> The 'select' is definitely misplaced here, that doesn't even work when >>>> the dependencies fo 8250 itself are not met, and it does force-enable >>>> the entire TTY subsystem. >>> >>> So, something like the following (untested) patch? >>> I admit to not fully understanding all the nuances around Kconfig. >> >> Hi, >> >> You can also remove the "select NET_DEVLINK". The driver builds fine >> without it. And please drop the "default n" while at it. > > I had to add this one because devlink is a dependency and the kbuild > robot generated a config without it. What kind of dependency is devlink? The driver builds without NET_DEVLINK. > The 'imply' statements were added because while the driver builds > without them, the resources don't show up unless the platform > modules are also present. This was really confusing users, since > they selected the OCP driver and then were not able to use the > flash since the XILINX modules had not been selected. > > Is there a better way of specifying these type of dependencies? Documentation/ and/or one can add comments/docs in the Kconfig help section. -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 20:45 ` Randy Dunlap @ 2021-08-25 21:14 ` Jonathan Lemon 2021-08-25 23:22 ` Randy Dunlap 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Lemon @ 2021-08-25 21:14 UTC (permalink / raw) To: Randy Dunlap; +Cc: Arnd Bergmann, Richard Cochran, Networking On Wed, Aug 25, 2021 at 01:45:57PM -0700, Randy Dunlap wrote: > On 8/25/21 1:40 PM, Jonathan Lemon wrote: > > On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote: > > > On 8/25/21 10:08 AM, Jonathan Lemon wrote: > > > > On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: > > > > > On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > > > > > > > On 8/20/21 8:31 AM, Richard Cochran wrote: > > > > > > > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: > > > > > > > > > > > > > > > I would also suggest removing all the 'imply' statements, they > > > > > > > > usually don't do what the original author intended anyway. > > > > > > > > If there is a compile-time dependency with those drivers, > > > > > > > > it should be 'depends on', otherwise they can normally be > > > > > > > > left out. > > > > > > > > > > > > > > +1 > > > > > > > > > > > > Hi, > > > > > > > > > > > > Removing the "imply" statements is simple enough and the driver > > > > > > still builds cleanly without them, so Yes, they aren't needed here. > > > > > > > > > > > > Removing the SPI dependency is also clean. > > > > > > > > > > > > The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they > > > > > > can't be removed without some other driver changes, like using > > > > > > #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. > > > > > > > > > > If the SERIAL_8250 dependency is actually required, then using > > > > > 'depends on' for this is probably better than an IS_ENABLED() check. > > > > > The 'select' is definitely misplaced here, that doesn't even work when > > > > > the dependencies fo 8250 itself are not met, and it does force-enable > > > > > the entire TTY subsystem. > > > > > > > > So, something like the following (untested) patch? > > > > I admit to not fully understanding all the nuances around Kconfig. > > > > > > Hi, > > > > > > You can also remove the "select NET_DEVLINK". The driver builds fine > > > without it. And please drop the "default n" while at it. > > > > I had to add this one because devlink is a dependency and the kbuild > > robot generated a config without it. > > What kind of dependency is devlink? > The driver builds without NET_DEVLINK. It really doesn't. Odds are one of the network drivers is also selecting this as well, so it is hidden. -- Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ptp: ocp: don't allow on S390 2021-08-25 21:14 ` Jonathan Lemon @ 2021-08-25 23:22 ` Randy Dunlap 0 siblings, 0 replies; 19+ messages in thread From: Randy Dunlap @ 2021-08-25 23:22 UTC (permalink / raw) To: Jonathan Lemon; +Cc: Arnd Bergmann, Richard Cochran, Networking On 8/25/21 2:14 PM, Jonathan Lemon wrote: > On Wed, Aug 25, 2021 at 01:45:57PM -0700, Randy Dunlap wrote: >> On 8/25/21 1:40 PM, Jonathan Lemon wrote: >>> On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote: >>>> On 8/25/21 10:08 AM, Jonathan Lemon wrote: >>>>> On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote: >>>>>> On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>>>>>> >>>>>>> On 8/20/21 8:31 AM, Richard Cochran wrote: >>>>>>>> On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote: >>>>>>>> >>>>>>>>> I would also suggest removing all the 'imply' statements, they >>>>>>>>> usually don't do what the original author intended anyway. >>>>>>>>> If there is a compile-time dependency with those drivers, >>>>>>>>> it should be 'depends on', otherwise they can normally be >>>>>>>>> left out. >>>>>>>> >>>>>>>> +1 >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Removing the "imply" statements is simple enough and the driver >>>>>>> still builds cleanly without them, so Yes, they aren't needed here. >>>>>>> >>>>>>> Removing the SPI dependency is also clean. >>>>>>> >>>>>>> The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they >>>>>>> can't be removed without some other driver changes, like using >>>>>>> #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs. >>>>>> >>>>>> If the SERIAL_8250 dependency is actually required, then using >>>>>> 'depends on' for this is probably better than an IS_ENABLED() check. >>>>>> The 'select' is definitely misplaced here, that doesn't even work when >>>>>> the dependencies fo 8250 itself are not met, and it does force-enable >>>>>> the entire TTY subsystem. >>>>> >>>>> So, something like the following (untested) patch? >>>>> I admit to not fully understanding all the nuances around Kconfig. >>>> >>>> Hi, >>>> >>>> You can also remove the "select NET_DEVLINK". The driver builds fine >>>> without it. And please drop the "default n" while at it. >>> >>> I had to add this one because devlink is a dependency and the kbuild >>> robot generated a config without it. >> >> What kind of dependency is devlink? >> The driver builds without NET_DEVLINK. > > It really doesn't. Odds are one of the network drivers is also > selecting this as well, so it is hidden. > OK, my mistake. Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-08-25 23:22 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-13 20:30 [PATCH] ptp: ocp: don't allow on S390 Randy Dunlap 2021-08-16 10:20 ` patchwork-bot+netdevbpf 2021-08-16 21:09 ` Jonathan Lemon 2021-08-16 21:15 ` Randy Dunlap 2021-08-16 21:41 ` Jonathan Lemon 2021-08-19 22:58 ` Randy Dunlap 2021-08-20 8:02 ` Christian Borntraeger 2021-08-20 9:59 ` Niklas Schnelle 2021-08-20 10:45 ` Arnd Bergmann 2021-08-20 15:31 ` Richard Cochran 2021-08-24 21:48 ` Randy Dunlap 2021-08-25 10:55 ` Arnd Bergmann 2021-08-25 17:08 ` Jonathan Lemon 2021-08-25 17:29 ` Randy Dunlap 2021-08-25 18:05 ` Arnd Bergmann 2021-08-25 20:40 ` Jonathan Lemon 2021-08-25 20:45 ` Randy Dunlap 2021-08-25 21:14 ` Jonathan Lemon 2021-08-25 23:22 ` Randy Dunlap
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.