Re: [PATCH] ptp: ocp: don't allow on S390

Re: [PATCH] ptp: ocp: don't allow on S390

[Christian Borntraeger]

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

Re: [PATCH] ptp: ocp: don't allow on S390

[Niklas Schnelle]

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