* Re: [PATCH 1/2] serdev: Add ACPI support
From: Marcel Holtmann @ 2017-10-06 18:32 UTC (permalink / raw)
To: Rob Herring
Cc: Frédéric Danis, Sebastian Reichel, Loic Poulain,
Johan Hovold, Lukas Wunner, Hans de Goede, Greg Kroah-Hartman,
open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKDzR9-ptE=SbL0LuQvTKDNT-GZ8buOvffJDyWz6fHfSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
>> This patch allows SerDev module to manage serial devices declared as
>> attached to an UART in ACPI table.
>>
>> acpi_serdev_add_device() callback will only take into account entries
>> without enumerated flag set. This flags is set for all entries during
>> ACPI scan, except for SPI and I2C serial devices, and for UART with
>> 2nd patch in the series.
>>
>> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 94 insertions(+), 5 deletions(-)
>
> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
so how do we get these changes upstream? If the serdev changes alone do not cause any harm, I am almost proposing taking them through bluetooth-next tree and only leave only the ACPI change to the ACPI maintainers.
Greg, any thoughts?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Marcel Holtmann @ 2017-10-06 18:33 UTC (permalink / raw)
To: Hans de Goede
Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
Sebastian Reichel, Rob Herring,
bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
linux-acpi
In-Reply-To: <2487535c-18b4-c689-3c67-d1261b21feee@redhat.com>
Hi Hans,
>>>>> Make the serdev driver use struct bcm_device as its driver data and share
>>>>> all the pm / GPIO / IRQ related code paths with the platform driver.
>>>>>
>>>>> After this commit the 2 drivers are in essence the same and the serdev
>>>>> driver interface can be used for all ACPI enumerated HCI UARTs.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
>>>>> 1 file changed, 68 insertions(+), 50 deletions(-)
>>>> all 9 patches have been applied to bluetooth-next tree.
>>>
>>> Excellent, thank you!
>>>
>>> So I guess this means we can also move forward with getting
>>> the 2 patches from Frédéric Danis merged ? There is a bit
>>> of a bisect-ability problem there, if the acpi pull-req
>>> gets merged first then uart attached bcm bt will stop
>>> working until the bluetooth subsys is also merged.
>>>
>>> But I don't think this will impact a lot of users
>>> (also given the need for a manual btattach so far),
>>> so I don't think this is a big problem... ?
>> I wonder if we should just do this as all-in-one change for 4.15 kernel. We surely can get the ACPI changes into 4.15 and the Bluetooth changes as well. Then it should just work.
>> So I am leaning to just adding the revert patch into the bluetooth-next tree and then just add the ACPI PnP ID for the GPD device in the same pull request. And if the ACPI changes make it then nobody will know the difference that we switched from USB to UART.
>
> Yes that should work fine.
I think that is what I am going for. Mainly since the GPD USB hack already seems to be in 4.13 and thus a little to late to fix anyway.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 09/16] serial: mvebu-uart: add TX interrupt trigger for pulse interrupts
From: Gregory CLEMENT @ 2017-10-06 20:22 UTC (permalink / raw)
To: Miquel Raynal
Cc: Greg Kroah-Hartman, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Jiri Slaby, Catalin Marinas, Will Deacon,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
Antoine Tenart, Nadav Haklai, Wilson Ding, Allen Yan
In-Reply-To: <20171006101344.15590-10-miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Miquel,
On ven., oct. 06 2017, Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> From: Allen Yan <yanwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> Pulse interrupts (extended UART only) needs a change of state to trigger
> the TX interrupt. In addition to enabling the TX_READY_INT_EN flag,
> produce a FIFO state change from 'empty' to 'not full'. For this, write
> only one data byte in TX start, making the TX FIFO not empty, and wait
> for the TX interrupt to continue the transfer.
>
> Signed-off-by: Allen Yan <yanwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Miquel Raynal <miquel.raynal-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Reviewed-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Thanks,
Gregory
> ---
> drivers/tty/serial/mvebu-uart.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
> index 67f302748b78..46d10209637a 100644
> --- a/drivers/tty/serial/mvebu-uart.c
> +++ b/drivers/tty/serial/mvebu-uart.c
> @@ -165,8 +165,16 @@ static void mvebu_uart_stop_tx(struct uart_port *port)
>
> static void mvebu_uart_start_tx(struct uart_port *port)
> {
> - unsigned int ctl = readl(port->membase + UART_INTR(port));
> + unsigned int ctl;
> + struct circ_buf *xmit = &port->state->xmit;
>
> + if (IS_EXTENDED(port) && !uart_circ_empty(xmit)) {
> + writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> + port->icount.tx++;
> + }
> +
> + ctl = readl(port->membase + UART_INTR(port));
> ctl |= CTRL_TX_RDY_INT(port);
> writel(ctl, port->membase + UART_INTR(port));
> }
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Rafael J. Wysocki @ 2017-10-07 0:03 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Rob Herring, Frédéric Danis, Sebastian Reichel,
Loic Poulain, Johan Hovold, Lukas Wunner, Hans de Goede,
Greg Kroah-Hartman, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, linux-acpi@vger.kernel.org
In-Reply-To: <BF8AC3FB-72FE-41C3-BE8D-27A32B84F6CB@holtmann.org>
On Fri, Oct 6, 2017 at 8:32 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Rob,
>
>>> This patch allows SerDev module to manage serial devices declared as
>>> attached to an UART in ACPI table.
>>>
>>> acpi_serdev_add_device() callback will only take into account entries
>>> without enumerated flag set. This flags is set for all entries during
>>> ACPI scan, except for SPI and I2C serial devices, and for UART with
>>> 2nd patch in the series.
>>>
>>> Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
>>> ---
>>> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 94 insertions(+), 5 deletions(-)
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>
> so how do we get these changes upstream? If the serdev changes alone do not cause any harm, I am almost proposing taking them through bluetooth-next tree and only leave only the ACPI change to the ACPI maintainers.
That would be fine by me. I can take the serdev patch too, though.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Marcel Holtmann @ 2017-10-07 0:31 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rob Herring, Frédéric Danis, Sebastian Reichel,
Loic Poulain, Johan Hovold, Lukas Wunner, Hans de Goede,
Greg Kroah-Hartman, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAJZ5v0gLhnisMn9o00ndnB6fjHt5V7KCy_57UScF=ZfZVF=dxA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rafael,
>>>> This patch allows SerDev module to manage serial devices declared as
>>>> attached to an UART in ACPI table.
>>>>
>>>> acpi_serdev_add_device() callback will only take into account entries
>>>> without enumerated flag set. This flags is set for all entries during
>>>> ACPI scan, except for SPI and I2C serial devices, and for UART with
>>>> 2nd patch in the series.
>>>>
>>>> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
>>>> 1 file changed, 94 insertions(+), 5 deletions(-)
>>>
>>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>
>> so how do we get these changes upstream? If the serdev changes alone do not cause any harm, I am almost proposing taking them through bluetooth-next tree and only leave only the ACPI change to the ACPI maintainers.
>
> That would be fine by me. I can take the serdev patch too, though.
having both patches go via ACPI tree might be simplest. Greg, any objections from you?
Regards
Marcel
^ permalink raw reply
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
From: Troy Kisky @ 2017-10-07 1:46 UTC (permalink / raw)
To: gregkh
Cc: Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
drivers/tty/serial/imx.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfeff3951f93..15b0ecb4cf60 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
}
static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
- unsigned long temp;
- unsigned long flags;
-
- spin_lock_irqsave(&sport->port.lock, flags);
-
- temp = readl(sport->port.membase + USR2);
- if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
- imx_disable_rx_int(sport);
-
- /* tell the DMA to receive the data. */
- start_rx_dma(sport);
- }
-
- spin_unlock_irqrestore(&sport->port.lock, flags);
-}
/*
* We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
sts = readl(sport->port.membase + USR1);
sts2 = readl(sport->port.membase + USR2);
- if (sts & (USR1_RRDY | USR1_AGTIM)) {
- if (sport->dma_is_enabled)
- imx_dma_rxint(sport);
- else
- imx_rxint(irq, dev_id);
+ if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+ imx_rxint(irq, dev_id);
ret = IRQ_HANDLED;
}
@@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port *sport)
temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
writel(temp, sport->port.membase + UCR1);
- temp = readl(sport->port.membase + UCR2);
- temp |= UCR2_ATEN;
- writel(temp, sport->port.membase + UCR2);
-
imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
sport->dma_is_enabled = 1;
--
2.11.0
^ permalink raw reply related
* [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Ian W MORRISON @ 2017-10-07 6:16 UTC (permalink / raw)
To: marcel, gustavo, johan.hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org)
Cc: Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
linux-serial, linux-acpi, Greg Kroah-Hartman, Rafael J. Wysocki
The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
---
drivers/tty/serdev/Kconfig | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index cdc6b820cf93..a9fb09a9c105 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -2,7 +2,8 @@
# Serial bus device driver configuration
#
menuconfig SERIAL_DEV_BUS
- tristate "Serial device bus"
+ bool "Serial device bus"
+ default y
help
Core support for devices connected via a serial port.
@@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
config SERIAL_DEV_CTRL_TTYPORT
bool "Serial device TTY port controller"
depends on TTY
- depends on SERIAL_DEV_BUS != m
+ default y
endif
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Greg Kroah-Hartman @ 2017-10-07 6:41 UTC (permalink / raw)
To: Ian W MORRISON
Cc: marcel-kz+m5ild9QBg9hUCZPvPmw, gustavo-THi1TnShQwVAfugRpC6u6w,
johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w,
bluez mailin list (linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Rafael J. Wysocki
In-Reply-To: <a4051ac2-1156-751d-ec16-2dbbb30b9258-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sat, Oct 07, 2017 at 05:16:31PM +1100, Ian W MORRISON wrote:
> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
> ---
> drivers/tty/serdev/Kconfig | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> index cdc6b820cf93..a9fb09a9c105 100644
> --- a/drivers/tty/serdev/Kconfig
> +++ b/drivers/tty/serdev/Kconfig
> @@ -2,7 +2,8 @@
> # Serial bus device driver configuration
> #
> menuconfig SERIAL_DEV_BUS
> - tristate "Serial device bus"
> + bool "Serial device bus"
> + default y
> help
> Core support for devices connected via a serial port.
>
> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
> config SERIAL_DEV_CTRL_TTYPORT
> bool "Serial device TTY port controller"
> depends on TTY
> - depends on SERIAL_DEV_BUS != m
> + default y
>
> endif
> --
> 2.11.0
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch contains warnings and/or errors noticed by the
scripts/checkpatch.pl tool.
- Your patch does not have a Signed-off-by: line. Please read the
kernel file, Documentation/SubmittingPatches and resend it after
adding that line. Note, the line needs to be in the body of the
email, before the patch, not at the bottom of the patch or in the
email signature.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Greg Kroah-Hartman @ 2017-10-07 6:42 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Rafael J. Wysocki, Rob Herring, Frédéric Danis,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
Hans de Goede, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <E5446B94-9914-44B5-A734-050F7457746D-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On Sat, Oct 07, 2017 at 02:31:05AM +0200, Marcel Holtmann wrote:
> Hi Rafael,
>
> >>>> This patch allows SerDev module to manage serial devices declared as
> >>>> attached to an UART in ACPI table.
> >>>>
> >>>> acpi_serdev_add_device() callback will only take into account entries
> >>>> without enumerated flag set. This flags is set for all entries during
> >>>> ACPI scan, except for SPI and I2C serial devices, and for UART with
> >>>> 2nd patch in the series.
> >>>>
> >>>> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>> ---
> >>>> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
> >>>> 1 file changed, 94 insertions(+), 5 deletions(-)
> >>>
> >>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>
> >> so how do we get these changes upstream? If the serdev changes alone do not cause any harm, I am almost proposing taking them through bluetooth-next tree and only leave only the ACPI change to the ACPI maintainers.
> >
> > That would be fine by me. I can take the serdev patch too, though.
>
> having both patches go via ACPI tree might be simplest. Greg, any objections from you?
None from me, let me go ack the serdev patch...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Marcel Holtmann @ 2017-10-07 6:42 UTC (permalink / raw)
To: Ian W MORRISON
Cc: Gustavo F. Padovan, Johan Hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
linux-serial, linux-acpi, Greg Kroah-Hartman, Rafael J. Wysocki
In-Reply-To: <a4051ac2-1156-751d-ec16-2dbbb30b9258@gmail.com>
Hi Ian,
> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
> ---
> drivers/tty/serdev/Kconfig | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> index cdc6b820cf93..a9fb09a9c105 100644
> --- a/drivers/tty/serdev/Kconfig
> +++ b/drivers/tty/serdev/Kconfig
> @@ -2,7 +2,8 @@
> # Serial bus device driver configuration
> #
> menuconfig SERIAL_DEV_BUS
> - tristate "Serial device bus"
> + bool "Serial device bus"
> + default y
> help
> Core support for devices connected via a serial port.
>
> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
> config SERIAL_DEV_CTRL_TTYPORT
> bool "Serial device TTY port controller"
> depends on TTY
> - depends on SERIAL_DEV_BUS != m
> + default y
actually we made hci_nokia.c be buildable as separate module. We might need to do the same for hci_bcm.c to avoid forcing SERIAL_DEV_BUS into a bool. Or you need to explain the reason behind this change a bit better.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Greg KH @ 2017-10-07 6:42 UTC (permalink / raw)
To: Frédéric Danis
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, marcel-kz+m5ild9QBg9hUCZPvPmw,
sre-DgEjT+Ai2ygdnm+yROfE0A, loic.poulain-Re5JQEeQqe8AvxtiuMwx3w,
johan-DgEjT+Ai2ygdnm+yROfE0A, lukas-JFq808J9C/izQB+pC5nmwQ,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1507107090-15992-2-git-send-email-frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Oct 04, 2017 at 10:51:29AM +0200, Frédéric Danis wrote:
> This patch allows SerDev module to manage serial devices declared as
> attached to an UART in ACPI table.
>
> acpi_serdev_add_device() callback will only take into account entries
> without enumerated flag set. This flags is set for all entries during
> ACPI scan, except for SPI and I2C serial devices, and for UART with
> 2nd patch in the series.
>
> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Sebastian Reichel @ 2017-10-07 11:35 UTC (permalink / raw)
To: Frédéric Danis
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, marcel-kz+m5ild9QBg9hUCZPvPmw,
loic.poulain-Re5JQEeQqe8AvxtiuMwx3w, johan-DgEjT+Ai2ygdnm+yROfE0A,
lukas-JFq808J9C/izQB+pC5nmwQ, hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1507107090-15992-2-git-send-email-frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5241 bytes --]
Hi,
On Wed, Oct 04, 2017 at 10:51:29AM +0200, Frédéric Danis wrote:
> This patch allows SerDev module to manage serial devices declared as
> attached to an UART in ACPI table.
>
> acpi_serdev_add_device() callback will only take into account entries
> without enumerated flag set. This flags is set for all entries during
> ACPI scan, except for SPI and I2C serial devices, and for UART with
> 2nd patch in the series.
>
> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
-- Sebastian
> ---
> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 94 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index c68fb3a..104777d 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -14,6 +14,7 @@
> * GNU General Public License for more details.
> */
>
> +#include <linux/acpi.h>
> #include <linux/errno.h>
> #include <linux/idr.h>
> #include <linux/kernel.h>
> @@ -49,13 +50,22 @@ static const struct device_type serdev_ctrl_type = {
>
> static int serdev_device_match(struct device *dev, struct device_driver *drv)
> {
> - /* TODO: ACPI and platform matching */
> + /* TODO: platform matching */
> + if (acpi_driver_match_device(dev, drv))
> + return 1;
> +
> return of_driver_match_device(dev, drv);
> }
>
> static int serdev_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> - /* TODO: ACPI and platform modalias */
> + int rc;
> +
> + /* TODO: platform modalias */
> + rc = acpi_device_uevent_modalias(dev, env);
> + if (rc != -ENODEV)
> + return rc;
> +
> return of_device_uevent_modalias(dev, env);
> }
>
> @@ -260,6 +270,12 @@ static int serdev_drv_remove(struct device *dev)
> static ssize_t modalias_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> + int len;
> +
> + len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
> + if (len != -ENODEV)
> + return len;
> +
> return of_device_modalias(dev, buf, PAGE_SIZE);
> }
> DEVICE_ATTR_RO(modalias);
> @@ -385,6 +401,74 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
> + struct acpi_device *adev)
> +{
> + struct serdev_device *serdev = NULL;
> + int err;
> +
> + if (acpi_bus_get_status(adev) || !adev->status.present ||
> + acpi_device_enumerated(adev))
> + return AE_OK;
> +
> + serdev = serdev_device_alloc(ctrl);
> + if (!serdev) {
> + dev_err(&ctrl->dev, "failed to allocate Serial device for %s\n",
> + dev_name(&adev->dev));
> + return AE_NO_MEMORY;
> + }
> +
> + ACPI_COMPANION_SET(&serdev->dev, adev);
> + acpi_device_set_enumerated(adev);
> +
> + err = serdev_device_add(serdev);
> + if (err) {
> + dev_err(&serdev->dev,
> + "failure adding ACPI device. status %d\n", err);
> + serdev_device_put(serdev);
> + }
> +
> + return AE_OK;
> +}
> +
> +static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
> + void *data, void **return_value)
> +{
> + struct serdev_controller *ctrl = data;
> + struct acpi_device *adev;
> +
> + if (acpi_bus_get_device(handle, &adev))
> + return AE_OK;
> +
> + return acpi_serdev_register_device(ctrl, adev);
> +}
> +
> +static int acpi_serdev_register_devices(struct serdev_controller *ctrl)
> +{
> + acpi_status status;
> + acpi_handle handle;
> +
> + handle = ACPI_HANDLE(ctrl->dev.parent);
> + if (!handle)
> + return -ENODEV;
> +
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> + acpi_serdev_add_device, NULL, ctrl, NULL);
> + if (ACPI_FAILURE(status)) {
> + dev_warn(&ctrl->dev, "failed to enumerate Serial slaves\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +#else
> +static inline int acpi_serdev_register_devices(struct serdev_controller *ctlr)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_ACPI */
> +
> /**
> * serdev_controller_add() - Add an serdev controller
> * @ctrl: controller to be registered.
> @@ -394,7 +478,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> */
> int serdev_controller_add(struct serdev_controller *ctrl)
> {
> - int ret;
> + int ret_of, ret_acpi, ret;
>
> /* Can't register until after driver model init */
> if (WARN_ON(!is_registered))
> @@ -404,9 +488,14 @@ int serdev_controller_add(struct serdev_controller *ctrl)
> if (ret)
> return ret;
>
> - ret = of_serdev_register_devices(ctrl);
> - if (ret)
> + ret_of = of_serdev_register_devices(ctrl);
> + ret_acpi = acpi_serdev_register_devices(ctrl);
> + if (ret_of && ret_acpi) {
> + dev_dbg(&ctrl->dev, "serdev%d no devices registered: of:%d acpi:%d\n",
> + ctrl->nr, ret_of, ret_acpi);
> + ret = -ENODEV;
> goto out_dev_del;
> + }
>
> dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n",
> ctrl->nr, &ctrl->dev);
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] ACPI / scan: Fix enumeration for special UART devices
From: Sebastian Reichel @ 2017-10-07 11:36 UTC (permalink / raw)
To: Frédéric Danis
Cc: robh, marcel, loic.poulain, johan, lukas, hdegoede,
linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <1507107090-15992-3-git-send-email-frederic.danis.oss@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]
Hi,
On Wed, Oct 04, 2017 at 10:51:30AM +0200, Frédéric Danis wrote:
> UART devices is expected to be enumerated by SerDev subsystem.
>
> During ACPI scan, serial devices behind SPI, I2C or UART buses are not
> enumerated, allowing them to be enumerated by their respective parents.
>
> Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
> devices on serial buses (SPI, I2C or UART).
>
> On Macs an empty ResourceTemplate is returned for uart slaves.
> Instead the device properties "baud", "parity", "dataBits", "stopBits" are
> provided. Add a check for "baud" in acpi_is_serial_bus_slave().
>
> Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
-- Sebastian
> drivers/acpi/scan.c | 37 +++++++++++++++++--------------------
> include/acpi/acpi_bus.h | 2 +-
> 2 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 602f8ff..860b698 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1505,41 +1505,38 @@ static void acpi_init_coherency(struct acpi_device *adev)
> adev->flags.coherent_dma = cca;
> }
>
> -static int acpi_check_spi_i2c_slave(struct acpi_resource *ares, void *data)
> +static int acpi_check_serial_bus_slave(struct acpi_resource *ares, void *data)
> {
> - bool *is_spi_i2c_slave_p = data;
> + bool *is_serial_bus_slave_p = data;
>
> if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
> return 1;
>
> - /*
> - * devices that are connected to UART still need to be enumerated to
> - * platform bus
> - */
> - if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART)
> - *is_spi_i2c_slave_p = true;
> + *is_serial_bus_slave_p = true;
>
> /* no need to do more checking */
> return -1;
> }
>
> -static bool acpi_is_spi_i2c_slave(struct acpi_device *device)
> +static bool acpi_is_serial_bus_slave(struct acpi_device *device)
> {
> struct list_head resource_list;
> - bool is_spi_i2c_slave = false;
> + bool is_serial_bus_slave = false;
>
> /* Macs use device properties in lieu of _CRS resources */
> if (x86_apple_machine &&
> (fwnode_property_present(&device->fwnode, "spiSclkPeriod") ||
> - fwnode_property_present(&device->fwnode, "i2cAddress")))
> + fwnode_property_present(&device->fwnode, "i2cAddress") ||
> + fwnode_property_present(&device->fwnode, "baud")))
> return true;
>
> INIT_LIST_HEAD(&resource_list);
> - acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave,
> - &is_spi_i2c_slave);
> + acpi_dev_get_resources(device, &resource_list,
> + acpi_check_serial_bus_slave,
> + &is_serial_bus_slave);
> acpi_dev_free_resource_list(&resource_list);
>
> - return is_spi_i2c_slave;
> + return is_serial_bus_slave;
> }
>
> void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
> @@ -1557,7 +1554,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
> acpi_bus_get_flags(device);
> device->flags.match_driver = false;
> device->flags.initialized = true;
> - device->flags.spi_i2c_slave = acpi_is_spi_i2c_slave(device);
> + device->flags.serial_bus_slave = acpi_is_serial_bus_slave(device);
> acpi_device_clear_enumerated(device);
> device_initialize(&device->dev);
> dev_set_uevent_suppress(&device->dev, true);
> @@ -1841,10 +1838,10 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
> static void acpi_default_enumeration(struct acpi_device *device)
> {
> /*
> - * Do not enumerate SPI/I2C slaves as they will be enumerated by their
> - * respective parents.
> + * Do not enumerate SPI/I2C/UART slaves as they will be enumerated by
> + * their respective parents.
> */
> - if (!device->flags.spi_i2c_slave) {
> + if (!device->flags.serial_bus_slave) {
> acpi_create_platform_device(device, NULL);
> acpi_device_set_enumerated(device);
> } else {
> @@ -1941,7 +1938,7 @@ static void acpi_bus_attach(struct acpi_device *device)
> return;
>
> device->flags.match_driver = true;
> - if (ret > 0 && !device->flags.spi_i2c_slave) {
> + if (ret > 0 && !device->flags.serial_bus_slave) {
> acpi_device_set_enumerated(device);
> goto ok;
> }
> @@ -1950,7 +1947,7 @@ static void acpi_bus_attach(struct acpi_device *device)
> if (ret < 0)
> return;
>
> - if (!device->pnp.type.platform_id && !device->flags.spi_i2c_slave)
> + if (!device->pnp.type.platform_id && !device->flags.serial_bus_slave)
> acpi_device_set_enumerated(device);
> else
> acpi_default_enumeration(device);
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index fa15052..f849be2 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -211,7 +211,7 @@ struct acpi_device_flags {
> u32 of_compatible_ok:1;
> u32 coherent_dma:1;
> u32 cca_seen:1;
> - u32 spi_i2c_slave:1;
> + u32 serial_bus_slave:1;
> u32 reserved:19;
> };
>
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Ian W MORRISON @ 2017-10-07 12:06 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Gustavo F. Padovan, Johan Hedberg,
bluez mailin list (linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
Rafael J. Wysocki
In-Reply-To: <D567DA4A-4443-4A88-B078-50EE458C822A-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On 7 October 2017 at 17:42, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Ian,
>
>> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
>> ---
>> drivers/tty/serdev/Kconfig | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
>> index cdc6b820cf93..a9fb09a9c105 100644
>> --- a/drivers/tty/serdev/Kconfig
>> +++ b/drivers/tty/serdev/Kconfig
>> @@ -2,7 +2,8 @@
>> # Serial bus device driver configuration
>> #
>> menuconfig SERIAL_DEV_BUS
>> - tristate "Serial device bus"
>> + bool "Serial device bus"
>> + default y
>> help
>> Core support for devices connected via a serial port.
>>
>> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
>> config SERIAL_DEV_CTRL_TTYPORT
>> bool "Serial device TTY port controller"
>> depends on TTY
>> - depends on SERIAL_DEV_BUS != m
>> + default y
>
> actually we made hci_nokia.c be buildable as separate module. We might need to do the same for hci_bcm.c to avoid forcing SERIAL_DEV_BUS into a bool. Or you need to explain the reason behind this change a bit better.
>
> Regards
>
> Marcel
>
Hi Marcel,
For serdev to support for devices connected via a serial port such as
a tty (for example an ACPI enumerated BT driver) its code hooks into
TTY code meaning serdev can no longer be compiled as a module for this
functionality to work. With CONFIG_SERIAL_DEV_BUS originally being
defined as a tristate a provision was made when compiling serdev as a
module to return ENODEV when trying to register a tty port. This
provision still exists with CONFIG_SERIAL_DEV_BUS defined as a bool
because CONFIG_SERIAL_DEV_CTRL_TTYPORT actually controls this.
My patch also aligns CONFIG_SERIAL_DEV_BUS (serdev) which provides
"Core serial port support" to CONFIG_SERIAL_NONSTANDARD or
"Non-standard serial port support" as this seems logical comparison.
CONFIG_SERIAL_NONSTANDARD is a bool similar to how I've made
CONFIG_SERIAL_DEV_BUS a bool.
Whilst it may be preferable to make hci_bcm.c buildable as separate
module like hci_nokia.c I don't relate the 'modularity' of bluetooth
drivers with the 'modularity' of a serial device bus as I see the
latter more similar to PCI support or ACPI support.
Because of the link and/or cross-over between serial port support and
tty support I believe serdev should be built-in rather than
modularized if and when it is required.
If this better explains the reason for the patch I can resubmit with
an updated changelog together with a signoff which I accidentally
during editing.
Regards,
Ian
^ permalink raw reply
* Re: [PATCH v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Johan Hovold @ 2017-10-07 14:26 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Hans de Goede, Gustavo F. Padovan, Johan Hedberg,
Frédéric Danis, Sebastian Reichel, Rob Herring,
bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
linux-acpi
In-Reply-To: <17592E57-3A4B-4386-B809-9EA928D38FDD@holtmann.org>
On Thu, Oct 05, 2017 at 05:15:05PM +0200, Marcel Holtmann wrote:
> Hi Hans,
>
> >>> Make the serdev driver use struct bcm_device as its driver data and share
> >>> all the pm / GPIO / IRQ related code paths with the platform driver.
> >>>
> >>> After this commit the 2 drivers are in essence the same and the serdev
> >>> driver interface can be used for all ACPI enumerated HCI UARTs.
> >>>
> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>> ---
> >>> drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
> >>> 1 file changed, 68 insertions(+), 50 deletions(-)
> >> all 9 patches have been applied to bluetooth-next tree.
> >
> > Excellent, thank you!
Nice work here, Hans, allowing serdev to coexist with the
platform-device hacks until the transition is complete and those hacks
can finally be removed.
> > So I guess this means we can also move forward with getting
> > the 2 patches from Frédéric Danis merged ? There is a bit
> > of a bisect-ability problem there, if the acpi pull-req
> > gets merged first then uart attached bcm bt will stop
> > working until the bluetooth subsys is also merged.
> >
> > But I don't think this will impact a lot of users
> > (also given the need for a manual btattach so far),
> > so I don't think this is a big problem... ?
>
> I wonder if we should just do this as all-in-one change for 4.15
> kernel. We surely can get the ACPI changes into 4.15 and the Bluetooth
> changes as well. Then it should just work.
However, there are of course a couple of caveats. Once Frederic's ACPI
patches land, there will be no more platform child devices. Unless
serdev support is then compiled in, this means that PM will break
(silently). And if serdev is enabled, of course the tty class device is
gone and hciattach (btattach) will fail, but I guess everyone is aware
of that issue by now.
Should BT_HCIUART_BCM start depending on SERIAL_DEV_CTRL_TTYPORT (when
ACPI is enabled) to avoid such silent breakage once ACPI-support is
merged?
Johan
^ permalink raw reply
* Re: [PATCH v2 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev
From: Johan Hovold @ 2017-10-07 14:36 UTC (permalink / raw)
To: Hans de Goede
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
Frédéric Danis, Sebastian Reichel, robh,
linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171004184343.7855-1-hdegoede@redhat.com>
On Wed, Oct 04, 2017 at 08:43:35PM +0200, Hans de Goede wrote:
> Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
> on hci_uart-s using serdev.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Also set RTS (Suggested-by: Sebastian Reichel <sre@kernel.org>)
> ---
> drivers/bluetooth/hci_ldisc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index a746627e784e..eec95019f15c 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -41,6 +41,7 @@
> #include <linux/ioctl.h>
> #include <linux/skbuff.h>
> #include <linux/firmware.h>
> +#include <linux/serdev.h>
I know this is already merged, but do we really want to add a dependency
on serdev from hci_ldisc.c?
There is already a helper function host_set_baudrate() in hci_bcm to
handle another case like this. If more drivers will need to support
both then these could be moved to a common header, but we should at
least try to be consistent here.
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -298,6 +299,12 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
> unsigned int set = 0;
> unsigned int clear = 0;
>
> + if (hu->serdev) {
> + serdev_device_set_flow_control(hu->serdev, !enable);
> + serdev_device_set_rts(hu->serdev, !enable);
The order here may matter; in the non-serdev case, rts is raised before
enabling flow control.
> + return;
> + }
> +
> if (enable) {
> /* Disable hardware flow control */
> ktermios = tty->termios;
Johan
^ permalink raw reply
* Re: [PATCH 1/2] serdev: Add ACPI support
From: Johan Hovold @ 2017-10-07 15:12 UTC (permalink / raw)
To: Frédéric Danis
Cc: robh, marcel, sre, loic.poulain, johan, lukas, hdegoede,
linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <1507107090-15992-2-git-send-email-frederic.danis.oss@gmail.com>
On Wed, Oct 04, 2017 at 10:51:29AM +0200, Frédéric Danis wrote:
> This patch allows SerDev module to manage serial devices declared as
> attached to an UART in ACPI table.
>
> acpi_serdev_add_device() callback will only take into account entries
> without enumerated flag set. This flags is set for all entries during
> ACPI scan, except for SPI and I2C serial devices, and for UART with
> 2nd patch in the series.
>
> Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
> ---
> drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 94 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index c68fb3a..104777d 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -385,6 +401,74 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
> + struct acpi_device *adev)
> +{
> + struct serdev_device *serdev = NULL;
> + int err;
> +
> + if (acpi_bus_get_status(adev) || !adev->status.present ||
> + acpi_device_enumerated(adev))
> + return AE_OK;
> +
> + serdev = serdev_device_alloc(ctrl);
> + if (!serdev) {
> + dev_err(&ctrl->dev, "failed to allocate Serial device for %s\n",
s/Serial/serdev/
> + dev_name(&adev->dev));
> + return AE_NO_MEMORY;
> + }
> +
> + ACPI_COMPANION_SET(&serdev->dev, adev);
> + acpi_device_set_enumerated(adev);
> +
> + err = serdev_device_add(serdev);
> + if (err) {
> + dev_err(&serdev->dev,
> + "failure adding ACPI device. status %d\n", err);
s/ACPI/ACPI serdev/?
> + serdev_device_put(serdev);
> + }
> +
> + return AE_OK;
> +}
> +
> +static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
> + void *data, void **return_value)
> +{
> + struct serdev_controller *ctrl = data;
> + struct acpi_device *adev;
> +
> + if (acpi_bus_get_device(handle, &adev))
> + return AE_OK;
> +
> + return acpi_serdev_register_device(ctrl, adev);
> +}
> +
> +static int acpi_serdev_register_devices(struct serdev_controller *ctrl)
> +{
> + acpi_status status;
> + acpi_handle handle;
> +
> + handle = ACPI_HANDLE(ctrl->dev.parent);
> + if (!handle)
> + return -ENODEV;
> +
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> + acpi_serdev_add_device, NULL, ctrl, NULL);
> + if (ACPI_FAILURE(status)) {
> + dev_warn(&ctrl->dev, "failed to enumerate Serial slaves\n");
s/Serial/serdev/
> + return -ENODEV;
> + }
> +
> + return 0;
What if there are no slaves defined? I'm not very familiar with the ACPI
helpers, but from a quick look it seems you'd then end up returning zero
here which would cause the serdev controller to be registered instead of
the tty-class device.
[ And if I'm mistaken, you do want to suppress that error message for
when there are no slaves defined. ]
> +}
> +#else
> +static inline int acpi_serdev_register_devices(struct serdev_controller *ctlr)
s/ctlr/ctrl/
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_ACPI */
> +
> /**
> * serdev_controller_add() - Add an serdev controller
> * @ctrl: controller to be registered.
> @@ -394,7 +478,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> */
> int serdev_controller_add(struct serdev_controller *ctrl)
> {
> - int ret;
> + int ret_of, ret_acpi, ret;
>
> /* Can't register until after driver model init */
> if (WARN_ON(!is_registered))
> @@ -404,9 +488,14 @@ int serdev_controller_add(struct serdev_controller *ctrl)
> if (ret)
> return ret;
>
> - ret = of_serdev_register_devices(ctrl);
> - if (ret)
> + ret_of = of_serdev_register_devices(ctrl);
> + ret_acpi = acpi_serdev_register_devices(ctrl);
> + if (ret_of && ret_acpi) {
> + dev_dbg(&ctrl->dev, "serdev%d no devices registered: of:%d acpi:%d\n",
"serdev%d" is redundant here as you're using dev_dbg (which will print
the device name).
> + ctrl->nr, ret_of, ret_acpi);
> + ret = -ENODEV;
> goto out_dev_del;
> + }
>
> dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n",
> ctrl->nr, &ctrl->dev);
Hmm, I see it's already used here. No need to follow that example
though.
Johan
^ permalink raw reply
* Re: [PATCH 0/2] ACPI serdev support
From: Johan Hovold @ 2017-10-07 15:14 UTC (permalink / raw)
To: Frédéric Danis
Cc: Ian W MORRISON, Marcel Holtmann, Rob Herring, Sebastian Reichel,
Loic Poulain, Johan Hovold, Lukas Wunner, Hans de Goede,
bluez mailin list (linux-bluetooth@vger.kernel.org), linux-serial,
linux-acpi, Greg Kroah-Hartman
In-Reply-To: <8645dbfb-ca09-340d-e610-6899a9b7dd8a@gmail.com>
On Fri, Oct 06, 2017 at 07:36:27PM +0200, Frédéric Danis wrote:
> I took a look at dmesg.txt and I did not find any trace related to serdev.
> On the T100, where ttyS4 is used for Bluetooth, I can see the following
> traces:
> [ 11.732347] serial serial0: allocated controller
> 0xffff880036229000 id 0
> [ 11.732470] serial serial0-0: device serial0-0 registered
> [ 11.732475] serial serial0: serdev0 registered: dev:ffff880036229000
> [ 11.732478] serial serial0: tty port ttyS4 registered
>
> If serdev registration failed you should at least get something like:
> serdev0 no devices registered: of:<> acpi:<>
Only with debugging enabled (it's a dev_dbg).
Johan
^ permalink raw reply
* Re: [PATCH 2/2] ACPI / scan: Fix enumeration for special UART devices
From: Johan Hovold @ 2017-10-07 15:19 UTC (permalink / raw)
To: Frédéric Danis
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, marcel-kz+m5ild9QBg9hUCZPvPmw,
sre-DgEjT+Ai2ygdnm+yROfE0A, loic.poulain-Re5JQEeQqe8AvxtiuMwx3w,
johan-DgEjT+Ai2ygdnm+yROfE0A, lukas-JFq808J9C/izQB+pC5nmwQ,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1507107090-15992-3-git-send-email-frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Oct 04, 2017 at 10:51:30AM +0200, Frédéric Danis wrote:
> UART devices is expected to be enumerated by SerDev subsystem.
>
> During ACPI scan, serial devices behind SPI, I2C or UART buses are not
> enumerated, allowing them to be enumerated by their respective parents.
>
> Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
> devices on serial buses (SPI, I2C or UART).
>
> On Macs an empty ResourceTemplate is returned for uart slaves.
> Instead the device properties "baud", "parity", "dataBits", "stopBits" are
> provided. Add a check for "baud" in acpi_is_serial_bus_slave().
>
> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
So just to reiterate what I just mentioned in a comment to one of Hans's
hci_bcm patches:
This one would silently break PM for such devices on any system which
does not have serdev enabled (as the corresponding platform devices
would no longer be registered). And with serdev enabled, hciattach
(btattach) would start failing as the tty device would no longer be
registered (but I assume everyone is aware of that, and fine with it, by
now).
Perhaps the hci_bcm driver should start depending on
SERIAL_DEV_CTRL_TTYPORT when ACPI is enabled?
Johan
^ permalink raw reply
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Johan Hovold @ 2017-10-07 15:24 UTC (permalink / raw)
To: Ian W MORRISON
Cc: marcel, gustavo, johan.hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Johan Hovold, Lukas Wunner,
linux-serial, linux-acpi, Greg Kroah-Hartman, Rafael J. Wysocki
In-Reply-To: <a4051ac2-1156-751d-ec16-2dbbb30b9258@gmail.com>
On Sat, Oct 07, 2017 at 05:16:31PM +1100, Ian W MORRISON wrote:
> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
> ---
> drivers/tty/serdev/Kconfig | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> index cdc6b820cf93..a9fb09a9c105 100644
> --- a/drivers/tty/serdev/Kconfig
> +++ b/drivers/tty/serdev/Kconfig
> @@ -2,7 +2,8 @@
> # Serial bus device driver configuration
> #
> menuconfig SERIAL_DEV_BUS
> - tristate "Serial device bus"
> + bool "Serial device bus"
> + default y
I understand why you want this (to prevent hci_bcm from breaking), but we
should generally not have new entries default to y.
> help
> Core support for devices connected via a serial port.
>
> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
> config SERIAL_DEV_CTRL_TTYPORT
> bool "Serial device TTY port controller"
> depends on TTY
> - depends on SERIAL_DEV_BUS != m
> + default y
Same here.
It may be better to have BT_HCIUART_BCM depend on (or select?)
SERIAL_DEV_CTRL_TTYPORT instead.
Johan
^ permalink raw reply
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Marcel Holtmann @ 2017-10-07 19:57 UTC (permalink / raw)
To: Johan Hovold
Cc: Ian W MORRISON, Gustavo F. Padovan, Johan Hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Lukas Wunner, linux-serial,
linux-acpi, Greg Kroah-Hartman, Rafael J. Wysocki
In-Reply-To: <20171007152414.GK2618@localhost>
Hi Johan,
>> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
>> ---
>> drivers/tty/serdev/Kconfig | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
>> index cdc6b820cf93..a9fb09a9c105 100644
>> --- a/drivers/tty/serdev/Kconfig
>> +++ b/drivers/tty/serdev/Kconfig
>> @@ -2,7 +2,8 @@
>> # Serial bus device driver configuration
>> #
>> menuconfig SERIAL_DEV_BUS
>> - tristate "Serial device bus"
>> + bool "Serial device bus"
>> + default y
>
> I understand why you want this (to prevent hci_bcm from breaking), but we
> should generally not have new entries default to y.
>
>> help
>> Core support for devices connected via a serial port.
>>
>> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
>> config SERIAL_DEV_CTRL_TTYPORT
>> bool "Serial device TTY port controller"
>> depends on TTY
>> - depends on SERIAL_DEV_BUS != m
>> + default y
>
> Same here.
>
> It may be better to have BT_HCIUART_BCM depend on (or select?)
> SERIAL_DEV_CTRL_TTYPORT instead.
if we move SERIAL_DEV_BUS to bool, then I would just have it be selected by BT_HCIUART_BCM. Frankly the SERIAL_DEV_BUS option is pretty hard to find in the kernel config. And if we depend on TTY, but then select SERIAL_DEV_BUS, I think that is a good compromise.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] ACPI / scan: Fix enumeration for special UART devices
From: Sebastian Reichel @ 2017-10-07 22:53 UTC (permalink / raw)
To: Johan Hovold
Cc: Frédéric Danis, robh, marcel, loic.poulain, lukas,
hdegoede, linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171007151934.GJ2618@localhost>
[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]
Hi,
On Sat, Oct 07, 2017 at 05:19:34PM +0200, Johan Hovold wrote:
> On Wed, Oct 04, 2017 at 10:51:30AM +0200, Frédéric Danis wrote:
> > UART devices is expected to be enumerated by SerDev subsystem.
> >
> > During ACPI scan, serial devices behind SPI, I2C or UART buses are not
> > enumerated, allowing them to be enumerated by their respective parents.
> >
> > Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
> > devices on serial buses (SPI, I2C or UART).
> >
> > On Macs an empty ResourceTemplate is returned for uart slaves.
> > Instead the device properties "baud", "parity", "dataBits", "stopBits" are
> > provided. Add a check for "baud" in acpi_is_serial_bus_slave().
> >
> > Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
>
> So just to reiterate what I just mentioned in a comment to one of Hans's
> hci_bcm patches:
>
> This one would silently break PM for such devices on any system which
> does not have serdev enabled (as the corresponding platform devices
> would no longer be registered). And with serdev enabled, hciattach
> (btattach) would start failing as the tty device would no longer be
> registered (but I assume everyone is aware of that, and fine with it, by
> now).
>
> Perhaps the hci_bcm driver should start depending on
> SERIAL_DEV_CTRL_TTYPORT when ACPI is enabled?
ACPI and DT both need SERIAL_DEV_CTRL_TTYPORT to work properly,
since SERIAL_DEV_CTRL_TTYPORT is the only controller implemented
for serdev. If any other controller is implemented that one could
also be used.
I wonder if we should just hide SERIAL_DEV_CTRL_TTYPORT and enable
it together with SERDEV. I suspect that we won't see any other
controller (it would be a UART device, that is not registered as
tty device) in the next few years and the extra option seems to
confuse people.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 1/2] serdev: Update drivers/tty/serdev/Kconfig for ACPI serdev support
From: Ian W MORRISON @ 2017-10-08 3:20 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hovold, Gustavo F. Padovan, Johan Hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org), hdegoede,
frederic.danis.oss, robh, sre, Loic Poulain, lukas, linux-serial,
linux-acpi, rafael, Greg Kroah-Hartman
ACPI and DT both need SERIAL_DEV_CTRL_TTYPORT to work properly since SERIAL_DEV_CTRL_TTYPORT is the only controller implemented for serdev. This is only possible if serdev support is compiled in as the code hooks into TTY. Otherwise PM will silently break as the corresponding platform devices would no longer be registered and as the tty class device is also gone and hciattach (btattach) will also fail.
This patch set addresses this by making BT_HCIUART_BCM dependent on SERIAL_DEV_CTRL_TTYPORT which in turn is dependent on SERIAL_DEV_BUS and ensures that if SERIAL_DEV_BUS is selected is the code is build it.
Signed-off-by: Ian W MORRISON <ianwmorrison@gmail.com>
---
drivers/tty/serdev/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index cdc6b820cf93..1144f4db5087 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -2,7 +2,7 @@
# Serial bus device driver configuration
#
menuconfig SERIAL_DEV_BUS
- tristate "Serial device bus"
+ bool "Serial device bus"
help
Core support for devices connected via a serial port.
@@ -11,6 +11,6 @@ if SERIAL_DEV_BUS
config SERIAL_DEV_CTRL_TTYPORT
bool "Serial device TTY port controller"
depends on TTY
- depends on SERIAL_DEV_BUS != m
+ depends on SERIAL_DEV_BUS
endif
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support
From: Ian W MORRISON @ 2017-10-08 3:20 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hovold, gustavo, johan.hedberg,
linux-bluetooth, Hans de Goede, frederic.danis.oss, robh, sre,
loic.poulain, Lukas Wunner, linux-serial, linux-acpi, rafael,
Greg Kroah-Hartman
ACPI and DT both need SERIAL_DEV_CTRL_TTYPORT to work properly since SERIAL_DEV_CTRL_TTYPORT is the only controller implemented for serdev. This is only possible if serdev support is compiled in as the code hooks into TTY. Otherwise PM will silently break as the corresponding platform devices would no longer be registered and as the tty class device is also gone and hciattach (btattach) will also fail.
This patch set addresses this by making BT_HCIUART_BCM dependent on SERIAL_DEV_CTRL_TTYPORT which in turn is dependent on SERIAL_DEV_BUS and ensures that if SERIAL_DEV_BUS is selected is the code is build it.
Signed-off-by: Ian W MORRISON <ianwmorrison@gmail.com>
---
drivers/bluetooth/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index fae5a74dc737..8d432ee9f4bd 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -171,6 +171,7 @@ config BT_HCIUART_BCM
depends on BT_HCIUART_SERDEV
select BT_HCIUART_H4
select BT_BCM
+ select SERIAL_DEV_CTRL_TTYPORT
help
The Broadcom protocol support enables Bluetooth HCI over serial
port interface for Broadcom Bluetooth controllers.
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support
From: Ian W MORRISON @ 2017-10-08 3:20 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hovold, Gustavo F. Padovan, Johan Hedberg,
bluez mailin list (linux-bluetooth@vger.kernel.org),
Hans de Goede, Frédéric Danis, Rob Herring,
Sebastian Reichel, Loic Poulain, Lukas Wunner, linux-serial,
linux-acpi, Greg Kroah-Hartman, Rafael J. Wysocki
In-Reply-To: <8D8274B0-4EF6-41A0-8B76-95023F6DD26C@holtmann.org>
On 10/8/17 6:57 AM, Marcel Holtmann wrote:
> Hi Johan,
>
>>> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
<snip>
>> I understand why you want this (to prevent hci_bcm from breaking), but we
>> should generally not have new entries default to y.
>>
<snip>
>> It may be better to have BT_HCIUART_BCM depend on (or select?)
>> SERIAL_DEV_CTRL_TTYPORT instead.
>
> if we move SERIAL_DEV_BUS to bool, then I would just have it be selected by BT_HCIUART_BCM. Frankly the SERIAL_DEV_BUS option is pretty hard to find in the kernel config. And if we depend on TTY, but then select SERIAL_DEV_BUS, I think that is a good compromise.
>
> Regards
>
> Marcel
>
Hi,
Many thanks for everyone's comments on my earlier patch to Kconfigs for ACPI serdev support.
I've submitted a revised patch set which addresses the points raised by making BT_HCIUART_BCM dependent on SERIAL_DEV_CTRL_TTYPORT which in turn is dependent on SERIAL_DEV_BUS and ensures that if SERIAL_DEV_BUS is selected is the code is build it.
Please can you review and let me know if any further changes are required?
Regards,
Ian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox