* [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports [not found] <CAL_JsqKNnfgESG6ON95D7nD8VNrcVy7-x6cGGnae_GbbGKAuPQ@mail.gmail.com> @ 2018-08-05 23:26 ` Andreas Färber 2018-08-06 16:21 ` Andreas Färber 2018-08-10 17:34 ` Rob Herring 0 siblings, 2 replies; 5+ messages in thread From: Andreas Färber @ 2018-08-05 23:26 UTC (permalink / raw) To: linux-serial, Rob Herring Cc: linux-mips, jringle, allsey87, Jakub Kicinski, Xue Liu, Andreas Färber, Greg Kroah-Hartman, Jiri Slaby, linux-kernel This is to allow using serdev. Signed-off-by: Andreas Färber <afaerber@suse.de> --- drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 243c96025053..ad7267274f65 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev, SC16IS7XX_IOCONTROL_SRESET_BIT); for (i = 0; i < devtype->nr_uart; ++i) { +#ifdef CONFIG_OF + struct device_node *np; + struct platform_device *pdev; + char name[6] = "uartx"; +#endif + s->p[i].line = i; /* Initialize port data */ +#ifdef CONFIG_OF + name[4] = '0' + i; + np = of_get_child_by_name(dev->of_node, name); + if (IS_ERR(np)) { + ret = PTR_ERR(np); + goto out_ports; + } + pdev = of_platform_device_create(np, NULL, dev); + if (IS_ERR(pdev)) { + ret = PTR_ERR(pdev); + goto out_ports; + } + platform_set_drvdata(pdev, dev_get_drvdata(dev)); + s->p[i].port.dev = &pdev->dev; +#else s->p[i].port.dev = dev; +#endif s->p[i].port.irq = irq; s->p[i].port.type = PORT_SC16IS7XX; s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; @@ -1271,6 +1293,9 @@ static int sc16is7xx_probe(struct device *dev, for (i--; i >= 0; i--) { uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port); clear_bit(s->p[i].port.line, &sc16is7xx_lines); +#ifdef CONFIG_OF + of_platform_device_destroy(s->p[i].port.dev, NULL); +#endif } #ifdef CONFIG_GPIOLIB -- 2.16.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports 2018-08-05 23:26 ` [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports Andreas Färber @ 2018-08-06 16:21 ` Andreas Färber 2018-08-10 17:34 ` Rob Herring 1 sibling, 0 replies; 5+ messages in thread From: Andreas Färber @ 2018-08-06 16:21 UTC (permalink / raw) To: linux-serial, Rob Herring Cc: linux-mips, jringle, allsey87, Jakub Kicinski, Xue Liu, Greg Kroah-Hartman, Jiri Slaby, linux-kernel Am 06.08.2018 um 01:26 schrieb Andreas Färber: > This is to allow using serdev. > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > index 243c96025053..ad7267274f65 100644 > --- a/drivers/tty/serial/sc16is7xx.c > +++ b/drivers/tty/serial/sc16is7xx.c > @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev, > SC16IS7XX_IOCONTROL_SRESET_BIT); > > for (i = 0; i < devtype->nr_uart; ++i) { > +#ifdef CONFIG_OF Looks like this and below need to be CONFIG_OF_ADDRESS (build failure reported for sparc). Regards, Andreas > + struct device_node *np; > + struct platform_device *pdev; > + char name[6] = "uartx"; > +#endif > + > s->p[i].line = i; > /* Initialize port data */ > +#ifdef CONFIG_OF > + name[4] = '0' + i; > + np = of_get_child_by_name(dev->of_node, name); > + if (IS_ERR(np)) { > + ret = PTR_ERR(np); > + goto out_ports; > + } > + pdev = of_platform_device_create(np, NULL, dev); > + if (IS_ERR(pdev)) { > + ret = PTR_ERR(pdev); > + goto out_ports; > + } > + platform_set_drvdata(pdev, dev_get_drvdata(dev)); > + s->p[i].port.dev = &pdev->dev; > +#else > s->p[i].port.dev = dev; > +#endif > s->p[i].port.irq = irq; > s->p[i].port.type = PORT_SC16IS7XX; > s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; > @@ -1271,6 +1293,9 @@ static int sc16is7xx_probe(struct device *dev, > for (i--; i >= 0; i--) { > uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port); > clear_bit(s->p[i].port.line, &sc16is7xx_lines); > +#ifdef CONFIG_OF > + of_platform_device_destroy(s->p[i].port.dev, NULL); > +#endif > } > > #ifdef CONFIG_GPIOLIB -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports 2018-08-05 23:26 ` [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports Andreas Färber 2018-08-06 16:21 ` Andreas Färber @ 2018-08-10 17:34 ` Rob Herring 2018-08-10 17:45 ` Andreas Färber 1 sibling, 1 reply; 5+ messages in thread From: Rob Herring @ 2018-08-10 17:34 UTC (permalink / raw) To: Andreas Färber Cc: open list:SERIAL DRIVERS, Linux-MIPS, jringle, Michael Allwright, Jakub Kicinski, liuxuenetmail, Greg Kroah-Hartman, Jiri Slaby, linux-kernel On Sun, Aug 5, 2018 at 5:27 PM Andreas Färber <afaerber@suse.de> wrote: > > This is to allow using serdev. > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > index 243c96025053..ad7267274f65 100644 > --- a/drivers/tty/serial/sc16is7xx.c > +++ b/drivers/tty/serial/sc16is7xx.c > @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev, > SC16IS7XX_IOCONTROL_SRESET_BIT); > > for (i = 0; i < devtype->nr_uart; ++i) { > +#ifdef CONFIG_OF > + struct device_node *np; > + struct platform_device *pdev; > + char name[6] = "uartx"; > +#endif > + > s->p[i].line = i; > /* Initialize port data */ > +#ifdef CONFIG_OF > + name[4] = '0' + i; > + np = of_get_child_by_name(dev->of_node, name); > + if (IS_ERR(np)) { > + ret = PTR_ERR(np); > + goto out_ports; > + } > + pdev = of_platform_device_create(np, NULL, dev); Ideally, you would use of_platform_default_populate here. I think you'd have to add a compatible to the child nodes, but that wouldn't be a bad thing. I could envision that the child nodes ultimately become their own driver utilizing the standard 8250 driver and a compatible string would be needed in that case. You'd then have to loop over each child of 'dev' instead of the DT nodes. > + if (IS_ERR(pdev)) { > + ret = PTR_ERR(pdev); > + goto out_ports; > + } > + platform_set_drvdata(pdev, dev_get_drvdata(dev)); > + s->p[i].port.dev = &pdev->dev; > +#else > s->p[i].port.dev = dev; > +#endif > s->p[i].port.irq = irq; > s->p[i].port.type = PORT_SC16IS7XX; > s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports 2018-08-10 17:34 ` Rob Herring @ 2018-08-10 17:45 ` Andreas Färber 2018-08-10 18:11 ` Rob Herring 0 siblings, 1 reply; 5+ messages in thread From: Andreas Färber @ 2018-08-10 17:45 UTC (permalink / raw) To: Rob Herring Cc: SERIAL DRIVERS, Linux-MIPS, jringle, Michael Allwright, Jakub Kicinski, liuxuenetmail, Greg Kroah-Hartman, Jiri Slaby, linux-kernel Am 10.08.2018 um 19:34 schrieb Rob Herring: > On Sun, Aug 5, 2018 at 5:27 PM Andreas Färber <afaerber@suse.de> wrote: >> >> This is to allow using serdev. >> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> --- >> drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c >> index 243c96025053..ad7267274f65 100644 >> --- a/drivers/tty/serial/sc16is7xx.c >> +++ b/drivers/tty/serial/sc16is7xx.c >> @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev, >> SC16IS7XX_IOCONTROL_SRESET_BIT); >> >> for (i = 0; i < devtype->nr_uart; ++i) { >> +#ifdef CONFIG_OF >> + struct device_node *np; >> + struct platform_device *pdev; >> + char name[6] = "uartx"; >> +#endif >> + >> s->p[i].line = i; >> /* Initialize port data */ >> +#ifdef CONFIG_OF >> + name[4] = '0' + i; >> + np = of_get_child_by_name(dev->of_node, name); >> + if (IS_ERR(np)) { >> + ret = PTR_ERR(np); >> + goto out_ports; >> + } >> + pdev = of_platform_device_create(np, NULL, dev); > > Ideally, you would use of_platform_default_populate here. I think > you'd have to add a compatible to the child nodes, but that wouldn't > be a bad thing. I could envision that the child nodes ultimately > become their own driver utilizing the standard 8250 driver and a > compatible string would be needed in that case. Separate compatibles would mean separate drivers. Unlike your DUART example this is not an MMIO device that we can easily split but a SPI slave (well, regmap due to some I2C models). I don't see how separate drivers could work, given that the whole spi_device has a single interrupt for all functions of this device. That left me with this ugly but working construct. Is the uartX naming correct, or should it be serialX? Regards, Andreas > > You'd then have to loop over each child of 'dev' instead of the DT nodes. > >> + if (IS_ERR(pdev)) { >> + ret = PTR_ERR(pdev); >> + goto out_ports; >> + } >> + platform_set_drvdata(pdev, dev_get_drvdata(dev)); >> + s->p[i].port.dev = &pdev->dev; >> +#else >> s->p[i].port.dev = dev; >> +#endif >> s->p[i].port.irq = irq; >> s->p[i].port.type = PORT_SC16IS7XX; >> s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports 2018-08-10 17:45 ` Andreas Färber @ 2018-08-10 18:11 ` Rob Herring 0 siblings, 0 replies; 5+ messages in thread From: Rob Herring @ 2018-08-10 18:11 UTC (permalink / raw) To: Andreas Färber Cc: open list:SERIAL DRIVERS, Linux-MIPS, jringle, Michael Allwright, Jakub Kicinski, liuxuenetmail, Greg Kroah-Hartman, Jiri Slaby, linux-kernel On Fri, Aug 10, 2018 at 11:45 AM Andreas Färber <afaerber@suse.de> wrote: > > Am 10.08.2018 um 19:34 schrieb Rob Herring: > > On Sun, Aug 5, 2018 at 5:27 PM Andreas Färber <afaerber@suse.de> wrote: > >> > >> This is to allow using serdev. > >> > >> Signed-off-by: Andreas Färber <afaerber@suse.de> > >> --- > >> drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++ > >> 1 file changed, 25 insertions(+) > >> > >> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > >> index 243c96025053..ad7267274f65 100644 > >> --- a/drivers/tty/serial/sc16is7xx.c > >> +++ b/drivers/tty/serial/sc16is7xx.c > >> @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev, > >> SC16IS7XX_IOCONTROL_SRESET_BIT); > >> > >> for (i = 0; i < devtype->nr_uart; ++i) { > >> +#ifdef CONFIG_OF > >> + struct device_node *np; > >> + struct platform_device *pdev; > >> + char name[6] = "uartx"; > >> +#endif > >> + > >> s->p[i].line = i; > >> /* Initialize port data */ > >> +#ifdef CONFIG_OF > >> + name[4] = '0' + i; > >> + np = of_get_child_by_name(dev->of_node, name); > >> + if (IS_ERR(np)) { > >> + ret = PTR_ERR(np); > >> + goto out_ports; > >> + } > >> + pdev = of_platform_device_create(np, NULL, dev); > > > > Ideally, you would use of_platform_default_populate here. I think > > you'd have to add a compatible to the child nodes, but that wouldn't > > be a bad thing. I could envision that the child nodes ultimately > > become their own driver utilizing the standard 8250 driver and a > > compatible string would be needed in that case. > > Separate compatibles would mean separate drivers. No. Having a compatible doesn't mean you have to have a driver. > Unlike your DUART example this is not an MMIO device that we can easily > split but a SPI slave (well, regmap due to some I2C models). A SPI slave could provide a regmap, right? > I don't see how separate drivers could work, given that the whole > spi_device has a single interrupt for all functions of this device. A shared interrupt or a parent driver that creates an irqchip like MFD drivers often do. > > That left me with this ugly but working construct. In any case, I'm was suggesting that you do any of this now. I just want the binding to be designed to work either way. > Is the uartX naming correct, or should it be serialX? Ah, yes. Should be serial@... I'm fine if both the parent and child are named serial@... Rob ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-10 18:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAL_JsqKNnfgESG6ON95D7nD8VNrcVy7-x6cGGnae_GbbGKAuPQ@mail.gmail.com>
2018-08-05 23:26 ` [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports Andreas Färber
2018-08-06 16:21 ` Andreas Färber
2018-08-10 17:34 ` Rob Herring
2018-08-10 17:45 ` Andreas Färber
2018-08-10 18:11 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).