From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yann E. MORIN <yann.morin.1998@free.fr>
Date: Mon, 19 Jul 2021 22:53:54 +0200
Subject: [Buildroot] [PATCH] board/raspberrypi: use kernel Bluetooth mode
In-Reply-To: <20210719122648.1ee4d094.john@metanate.com>
References: <20210714145132.533559-1-john@metanate.com>
 <20210718002242.4638b323@gmx.net>
 <20210719122648.1ee4d094.john@metanate.com>
Message-ID: <20210719205354.GC1464219@scaer>
List-Id: <buildroot.busybox.net>
To: buildroot@busybox.net
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

John, Peter, All,

On 2021-07-19 12:26 +0100, John Keeping spake thusly:
> On Sun, 18 Jul 2021 00:22:42 +0200
> Peter Seiderer <ps.report@gmx.net> wrote:
> > On Wed, 14 Jul 2021 15:51:32 +0100, John Keeping <john@metanate.com> wrote:
> > > The default setting with miniuart-bt requires hciattach which is a
> > > deprecated utility in BlueZ.  Setting the krnbt parameter switches to
> > > the modern method of using serdev in the kernel removing the need for
> > > any userspace configuration to enable the Bluetooth controller.
> > >
> > > This is documented as applying to all Raspberry Pi variants so just
> > > enable it globally.
> > >
> > > Signed-off-by: John Keeping <john@metanate.com>
> > > ---
> > >  board/raspberrypi/post-image.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/board/raspberrypi/post-image.sh b/board/raspberrypi/post-image.sh
> > > index 9dbd98ef9b..a6728c686e 100755
> > > --- a/board/raspberrypi/post-image.sh
> > > +++ b/board/raspberrypi/post-image.sh
> > > @@ -16,7 +16,7 @@ do
> > >  			cat << __EOF__ >> "${BINARIES_DIR}/rpi-firmware/config.txt"
> > >
> > >  # fixes rpi (3B, 3B+, 3A+, 4B and Zero W) ttyAMA0 serial console
> > > -dtoverlay=miniuart-bt
> > > +dtoverlay=miniuart-bt,krnbt=on
> > >  __EOF__
> > >  		fi
> > >  		;;  
> > 
> > I fully understand the aim of your patch, but it is beyond the the minimal
> > approach of the defconfigs (minimal as as starting point for own
> > enhancements - startup and at minimum serial console access if possible,
> > so even not all rpi defconfigs provide the DTB overlays) or as strong hint
> > your enhancement would not match the comment line above the changed line...
> 
> That comment is still correct - the serial console continues to work
> with krnbt=on set.  What doesn't work any more is hciattach on the
> Bluetooth serial device since that is no longer exposed and handled
> internally in the kernel.
> 
> I can understand why this isn't the default since RPi want Raspbian to
> continue to work in the hciattach configuration without any need to
> upgrade in step, but Buildroot doesn't have the same backwards
> compatibility requirements and the krnbt=on mode is more usable for
> modern systems since Bluetooth "just works" without any need for
> hciattach or other userspace configuration.

I was a bit undecided on the topic...

On one hand, I agree with Peter that we tend to have minimalist
configurations, while on the other hand we also like to have those
minimalist configuration to still enable the hardware present on the
boards, especially if there are tricks to do so.

Furthermore, we also like to provide configurations that use
state-of-the-art setups and technologies (for lack of a better word),
e.g. the recommened kernel-side handling in this case.

So, in the end, I think John's proposal does make sense.

However, krnbt is a generic parameter, for the "<The base DTB>". So,
it feels awkward to have it set as part of the --add-miniuart-bt-overlay
option... But this option applies to all rpis that havea BT chip, so I
can see the reasoning...

> > But for an alternative config.txt file handling (with better/easier support
> > for cutomizations) see [1]...
> > [1] https://patchwork.ozlabs.org/project/buildroot/patch/20210321114002.31000-1-ps.report at gmx.net/

Peter, could you please rebase and respin that series, please?

There has been no better proposal, especially from my part, so even if I
am still not 100% convinced, it does improve the situation by a fair
amount, so I will apply it.

> I missed that series when it was posted, but it does look like an
> improvement in configuring the system.  I will try to find some time to
> review it in depth.  But I do still think that whenever miniuart-bt is
> added as an overlay, the krnbt=on variant should be used.

I think this is the confusing part: miniuart-bt is restoring the UART on
the usual port, it is not enabling the BT chip. But in Buildroot, we use
miniuart-bt for all rpis that have a BT chip, so I can see how it fits
together *in our case*.

There is however one remaining question: is that kernel-side autorpobing
specific to the rpi (I guess not)? If not, I guess this is also driven
by a kernel cmdline parameter, no? And if so, shouldn't we use that
kernel cmdline parameter instead?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'