| Message ID | 1422334209-23125-2-git-send-email-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rob, On 01/26/2015 11:50 PM, Rob Herring wrote: > Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial > driver. These are 8250 variants which have a port type of PORT_XSCALE. > > There is also the serial driver pxa.c with these compatible strings > already. However, it can be replaced with the common 8250 driver. It has > some issues like it cannot coexist with the 8250 driver due to tty name > collision. That also means adding these compatible strings here should > not case a problem. So what determines which driver is controlling the port if both drivers are built-in? Regards, Peter Hurley > Signed-off-by: Rob Herring <robh@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jiri Slaby <jslaby@suse.cz> > Cc: linux-serial@vger.kernel.org > --- > drivers/tty/serial/of_serial.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c > index 3194b42..fbb719c 100644 > --- a/drivers/tty/serial/of_serial.c > +++ b/drivers/tty/serial/of_serial.c > @@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { > .data = (void *)PORT_ALTR_16550_F64, }, > { .compatible = "altr,16550-FIFO128", > .data = (void *)PORT_ALTR_16550_F128, }, > + { .compatible = "mrvl,mmp-uart", > + .data = (void *)PORT_XSCALE, }, > + { .compatible = "mrvl,pxa-uart", > + .data = (void *)PORT_XSCALE, }, > #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL > { .compatible = "ibm,qpace-nwp-serial", > .data = (void *)PORT_NWPSERIAL, }, >
On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > Hi Rob, > > On 01/26/2015 11:50 PM, Rob Herring wrote: >> Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial >> driver. These are 8250 variants which have a port type of PORT_XSCALE. >> >> There is also the serial driver pxa.c with these compatible strings >> already. However, it can be replaced with the common 8250 driver. It has >> some issues like it cannot coexist with the 8250 driver due to tty name >> collision. That also means adding these compatible strings here should >> not case a problem. > > So what determines which driver is controlling the port if both > drivers are built-in? If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order. So I guess PXA systems have avoided this by never building in 8250 driver. Rob > > Regards, > Peter Hurley > >> Signed-off-by: Rob Herring <robh@kernel.org> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Jiri Slaby <jslaby@suse.cz> >> Cc: linux-serial@vger.kernel.org >> --- >> drivers/tty/serial/of_serial.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c >> index 3194b42..fbb719c 100644 >> --- a/drivers/tty/serial/of_serial.c >> +++ b/drivers/tty/serial/of_serial.c >> @@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { >> .data = (void *)PORT_ALTR_16550_F64, }, >> { .compatible = "altr,16550-FIFO128", >> .data = (void *)PORT_ALTR_16550_F128, }, >> + { .compatible = "mrvl,mmp-uart", >> + .data = (void *)PORT_XSCALE, }, >> + { .compatible = "mrvl,pxa-uart", >> + .data = (void *)PORT_XSCALE, }, >> #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL >> { .compatible = "ibm,qpace-nwp-serial", >> .data = (void *)PORT_NWPSERIAL, }, >> >
On 01/27/2015 09:30 AM, Rob Herring wrote: > On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >> Hi Rob, >> >> On 01/26/2015 11:50 PM, Rob Herring wrote: >>> Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial >>> driver. These are 8250 variants which have a port type of PORT_XSCALE. >>> >>> There is also the serial driver pxa.c with these compatible strings >>> already. However, it can be replaced with the common 8250 driver. It has >>> some issues like it cannot coexist with the 8250 driver due to tty name >>> collision. That also means adding these compatible strings here should >>> not case a problem. >> >> So what determines which driver is controlling the port if both >> drivers are built-in? > > If both are built-in, whoever registers ttyS ports first wins. This > will be the 8250 driver due to link order. Ok, but then I think the commit log should reflect that this patch effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts if both drivers are built-in or both drivers are modules. > So I guess PXA systems have avoided this by never building in 8250 driver. Platform devices are initialized first, so before this patch the pxa2xx-uart driver would have claimed the platform ports before the 8250 driver, if both were built-in (or both modules). Maybe Kconfig should warn if they're both built-in or both modules? Regards, Peter Hurley >>> Signed-off-by: Rob Herring <robh@kernel.org> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Cc: Jiri Slaby <jslaby@suse.cz> >>> Cc: linux-serial@vger.kernel.org >>> --- >>> drivers/tty/serial/of_serial.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c >>> index 3194b42..fbb719c 100644 >>> --- a/drivers/tty/serial/of_serial.c >>> +++ b/drivers/tty/serial/of_serial.c >>> @@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { >>> .data = (void *)PORT_ALTR_16550_F64, }, >>> { .compatible = "altr,16550-FIFO128", >>> .data = (void *)PORT_ALTR_16550_F128, }, >>> + { .compatible = "mrvl,mmp-uart", >>> + .data = (void *)PORT_XSCALE, }, >>> + { .compatible = "mrvl,pxa-uart", >>> + .data = (void *)PORT_XSCALE, }, >>> #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL >>> { .compatible = "ibm,qpace-nwp-serial", >>> .data = (void *)PORT_NWPSERIAL, }, >>> >>
On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > On 01/27/2015 09:30 AM, Rob Herring wrote: >> On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>> Hi Rob, >>> >>> On 01/26/2015 11:50 PM, Rob Herring wrote: >>>> Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial >>>> driver. These are 8250 variants which have a port type of PORT_XSCALE. >>>> >>>> There is also the serial driver pxa.c with these compatible strings >>>> already. However, it can be replaced with the common 8250 driver. It has >>>> some issues like it cannot coexist with the 8250 driver due to tty name >>>> collision. That also means adding these compatible strings here should >>>> not case a problem. >>> >>> So what determines which driver is controlling the port if both >>> drivers are built-in? >> >> If both are built-in, whoever registers ttyS ports first wins. This >> will be the 8250 driver due to link order. > > Ok, but then I think the commit log should reflect that this patch > effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts > if both drivers are built-in or both drivers are modules. It doesn't always. It is only on DT enabled platforms. But I can be more explicit about that. There is another patch to remove the PXA driver entirely which I'm inquiring about why it was never merged [1]. This is why I went down the path of getting the 8250 driver to work rather than trying to fix the PXA driver. >> So I guess PXA systems have avoided this by never building in 8250 driver. > > Platform devices are initialized first, so before this patch the pxa2xx-uart > driver would have claimed the platform ports before the 8250 driver, if > both were built-in (or both modules). That's not exactly what I observed. The console is killed when the real driver initializes. I have no other debug output capability after that, so I'm not completely sure what was happening. Part of the problem is you need to move uart_register_driver to probe from the initcall as this is what other serial drivers sharing ttyS have done. Using 8250 earlycon with the PXA driver seems to cause problems, so I'd have to duplicate the earlycon support in the PXA driver. > Maybe Kconfig should warn if they're both built-in or both modules? Is there a way to do that? Rob [1] https://lkml.org/lkml/2015/1/19/471
On 01/27/2015 11:44 AM, Rob Herring wrote: > On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >> On 01/27/2015 09:30 AM, Rob Herring wrote: >>> On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>>> Hi Rob, >>>> >>>> On 01/26/2015 11:50 PM, Rob Herring wrote: >>>>> Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial >>>>> driver. These are 8250 variants which have a port type of PORT_XSCALE. >>>>> >>>>> There is also the serial driver pxa.c with these compatible strings >>>>> already. However, it can be replaced with the common 8250 driver. It has >>>>> some issues like it cannot coexist with the 8250 driver due to tty name >>>>> collision. That also means adding these compatible strings here should >>>>> not case a problem. >>>> >>>> So what determines which driver is controlling the port if both >>>> drivers are built-in? >>> >>> If both are built-in, whoever registers ttyS ports first wins. This >>> will be the 8250 driver due to link order. >> >> Ok, but then I think the commit log should reflect that this patch >> effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts >> if both drivers are built-in or both drivers are modules. > > It doesn't always. It is only on DT enabled platforms. But I can be > more explicit about that. > > There is another patch to remove the PXA driver entirely which I'm > inquiring about why it was never merged [1]. This is why I went down > the path of getting the 8250 driver to work rather than trying to fix > the PXA driver. Ah. I had seen your inquiry but didn't grasp the significance. Greg acked that patch over 8 mos ago; it's still stuck somewhere? OTOH, I didn't realize that patch removed the PXA driver. That driver has errata workarounds which didn't get integrated into the 8250 core; it seems a shame to toss that away. [ Sidebar: I did a trial split of 8250_core into a separate 8250 base module and an 8250 universal driver. I keep having to rework the split but it's pretty close now; the 8250 base module might be just the thing for simplifying the pxa driver and getting console/earlycon/dma for free. Let me see what I can do about getting that done. ] >>> So I guess PXA systems have avoided this by never building in 8250 driver. >> >> Platform devices are initialized first, so before this patch the pxa2xx-uart >> driver would have claimed the platform ports before the 8250 driver, if >> both were built-in (or both modules). > > That's not exactly what I observed. The console is killed when the > real driver initializes. I have no other debug output capability after > that, so I'm not completely sure what was happening. Part of the > problem is you need to move uart_register_driver to probe from the > initcall as this is what other serial drivers sharing ttyS have done. Ah, yep. I missed that subtlety. > Using 8250 earlycon with the PXA driver seems to cause problems, so > I'd have to duplicate the earlycon support in the PXA driver. > >> Maybe Kconfig should warn if they're both built-in or both modules? > > Is there a way to do that? I think so; conf warns if one setting overrides another, so I think there's probably a way to make that happen in the Kconfig. I'll mess with that later tonight or tomorrow, and get back to you. Regards, Peter Hurley
On Tue, Jan 27, 2015 at 1:43 PM, Peter Hurley <peter@hurleysoftware.com> wrote: > On 01/27/2015 11:44 AM, Rob Herring wrote: >> On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>> On 01/27/2015 09:30 AM, Rob Herring wrote: >>>> On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>>>> Hi Rob, >>>>> >>>>> On 01/26/2015 11:50 PM, Rob Herring wrote: >>>>>> Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial >>>>>> driver. These are 8250 variants which have a port type of PORT_XSCALE. >>>>>> >>>>>> There is also the serial driver pxa.c with these compatible strings >>>>>> already. However, it can be replaced with the common 8250 driver. It has >>>>>> some issues like it cannot coexist with the 8250 driver due to tty name >>>>>> collision. That also means adding these compatible strings here should >>>>>> not case a problem. >>>>> >>>>> So what determines which driver is controlling the port if both >>>>> drivers are built-in? >>>> >>>> If both are built-in, whoever registers ttyS ports first wins. This >>>> will be the 8250 driver due to link order. >>> >>> Ok, but then I think the commit log should reflect that this patch >>> effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts >>> if both drivers are built-in or both drivers are modules. >> >> It doesn't always. It is only on DT enabled platforms. But I can be >> more explicit about that. >> >> There is another patch to remove the PXA driver entirely which I'm >> inquiring about why it was never merged [1]. This is why I went down >> the path of getting the 8250 driver to work rather than trying to fix >> the PXA driver. > > Ah. I had seen your inquiry but didn't grasp the significance. > Greg acked that patch over 8 mos ago; it's still stuck somewhere? It appears to me that this patch got added as a dependency to the rest of the series which never got merged. Guess they lost interest. > OTOH, I didn't realize that patch removed the PXA driver. That driver has > errata workarounds which didn't get integrated into the 8250 core; it seems > a shame to toss that away. There was some discussion on errata in the prior versions. One of them was deemed not needed. Another one is needed, but that's on certain PXA2xx. So we'll need to pickup that work-around at some point, but it is not yet needed here because PXA2xx is not enabled for DT. > [ Sidebar: I did a trial split of 8250_core into a separate 8250 base module > and an 8250 universal driver. I keep having to rework the split but it's > pretty close now; the 8250 base module might be just the thing for simplifying > the pxa driver and getting console/earlycon/dma for free. > > Let me see what I can do about getting that done. > ] Yes, it would be nice to split the legacy ISA parts from the proper platform driver. There's way too many kconfig options for the 8250 as well IMO. BTW, It appears that the main motivator for a separate driver was to add DMA support and that's what the out of tree driver adds. But now, we would never add the PXA DMA API support and the 8250 driver has learned DMA support. Rob
On 01/28/2015 09:21 AM, Rob Herring wrote: > On Tue, Jan 27, 2015 at 1:43 PM, Peter Hurley <peter@hurleysoftware.com> wrote: >> On 01/27/2015 11:44 AM, Rob Herring wrote: >>> On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>>> On 01/27/2015 09:30 AM, Rob Herring wrote: >>>>> On Tue, Jan 27, 2015 at 6:44 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >>>>>> On 01/26/2015 11:50 PM, Rob Herring wrote: [...] >> OTOH, I didn't realize that patch removed the PXA driver. That driver has >> errata workarounds which didn't get integrated into the 8250 core; it seems >> a shame to toss that away. > > There was some discussion on errata in the prior versions. One of them > was deemed not needed. Another one is needed, but that's on certain > PXA2xx. So we'll need to pickup that work-around at some point, but it > is not yet needed here because PXA2xx is not enabled for DT. Do you know offhand which errata? If not, nbd; I can do a little digging. >> [ Sidebar: I did a trial split of 8250_core into a separate 8250 base module >> and an 8250 universal driver. I keep having to rework the split but it's >> pretty close now; the 8250 base module might be just the thing for simplifying >> the pxa driver and getting console/earlycon/dma for free. >> >> Let me see what I can do about getting that done. >> ] > > Yes, it would be nice to split the legacy ISA parts from the proper > platform driver. There's way too many kconfig options for the 8250 as > well IMO. After I finish the split, I can look at making most/all of those options dependent on the universal 8250 driver only. > BTW, It appears that the main motivator for a separate driver was to > add DMA support and that's what the out of tree driver adds. But now, > we would never add the PXA DMA API support and the 8250 driver has > learned DMA support. Yeah, PXA should not have been forked. But I say that with the benefit of hindsight. Regards, Peter Hurley
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c index 3194b42..fbb719c 100644 --- a/drivers/tty/serial/of_serial.c +++ b/drivers/tty/serial/of_serial.c @@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { .data = (void *)PORT_ALTR_16550_F64, }, { .compatible = "altr,16550-FIFO128", .data = (void *)PORT_ALTR_16550_F128, }, + { .compatible = "mrvl,mmp-uart", + .data = (void *)PORT_XSCALE, }, + { .compatible = "mrvl,pxa-uart", + .data = (void *)PORT_XSCALE, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL { .compatible = "ibm,qpace-nwp-serial", .data = (void *)PORT_NWPSERIAL, },
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial driver. These are 8250 variants which have a port type of PORT_XSCALE. There is also the serial driver pxa.c with these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem. Signed-off-by: Rob Herring <robh@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jslaby@suse.cz> Cc: linux-serial@vger.kernel.org --- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)