[PATCH v2 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[PATCH v2 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[Ian W MORRISON]

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

[PATCH v3 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[Ian W MORRISON]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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

Re: [PATCH v3 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[Johan Hovold]

On Mon, Oct 09, 2017 at 11:43:31AM +1100, Ian W MORRISON wrote:
> 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.

Ok, so you didn't even bother to write two distinct commit messages for
your two patches, and my comments to the first patch apply also here.

> Signed-off-by: Ian W MORRISON <ianwmorrison-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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

I think

	depends on (!ACPI || SERIAL_DEV_CTRL_TTYPORT)

or just

	depends on SERIAL_DEV_CTRL_TTYPORT

is what we want here, but that's still being discussed.

[ In general, select should be avoided as you could end up with options
enabled without their dependencies also being enabled. In this case,
we're fine as BT_HCIUART_BCM as depends on SERIAL_DEV_BUS via
BT_HCIUART_SERDEV, and your first patch ruled out SERIAL_DEV_BUS=y, but
that's not obvious. ]

>  	help
>  	  The Broadcom protocol support enables Bluetooth HCI over serial
>  	  port interface for Broadcom Bluetooth controllers.

Johan

Re: [PATCH v3 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[Ian W MORRISON]

On 9 October 2017 at 19:06, Johan Hovold <johan@kernel.org> wrote:
> On Mon, Oct 09, 2017 at 11:43:31AM +1100, Ian W MORRISON wrote:
<snip>
>>
>> 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.
>
> Ok, so you didn't even bother to write two distinct commit messages for
> your two patches, and my comments to the first patch apply also here.
>
<snip>
>
> Johan

Hi Johan,

My experience in submitting patches is still limited and I am very
much learning so thanks for the helpful comments and apologies for
trying your patience. My keenness on submitting patches was to show
that I had tested my suggestions so as to given them credibility
rather than any attempt to own the patch.

What I was trying to do here was to suggested two patches with one
requiring the other. Rather than keep as a single patch affecting two
files I separated into two patches attempting to create a patch series
with a single changelog. This was the wrong approach and I may have
misled so apologies for that.

Regards,
Ian

Re: [PATCH v3 2/2] serdev: Update drivers/bluetooth/Kconfig for ACPI serdev support

[Johan Hovold]

On Tue, Oct 10, 2017 at 06:16:46PM +1100, Ian W MORRISON wrote:
> On 9 October 2017 at 19:06, Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, Oct 09, 2017 at 11:43:31AM +1100, Ian W MORRISON wrote:
> <snip>
> >>
> >> 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.
> >
> > Ok, so you didn't even bother to write two distinct commit messages for
> > your two patches, and my comments to the first patch apply also here.
> >
> <snip>
> >
> > Johan
> 
> Hi Johan,
> 
> My experience in submitting patches is still limited and I am very
> much learning so thanks for the helpful comments and apologies for
> trying your patience. My keenness on submitting patches was to show
> that I had tested my suggestions so as to given them credibility
> rather than any attempt to own the patch.

No worries. I can recommend taking a closer look at

	Documentation/process/submitting-patches.rst

where some of the process is described.

> What I was trying to do here was to suggested two patches with one
> requiring the other. Rather than keep as a single patch affecting two
> files I separated into two patches attempting to create a patch series
> with a single changelog. This was the wrong approach and I may have
> misled so apologies for that.

So next, time just submit your patches as a series (consider using
git-send-email to get the threading right), where each patch is
self-contained and has a good commit message describing why that change
is needed. It is implicit that a series is to be applied in order and
that therefore later patches in a series can depend on earlier ones.
When a patch series span multiple subsystems, things can gets more
complicated, though...

Johan