Re: [PATCH] Bluetooth: qca: generalise device address check

From: Janaki Ramaiah Thota <quic_janathot@quicinc.com>
To: Johan Hovold <johan@kernel.org>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	Doug Anderson <dianders@chromium.org>,
	Johan Hovold <johan+linaro@kernel.org>,
	"Marcel Holtmann" <marcel@holtmann.org>,
	<linux-bluetooth@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<stable@vger.kernel.org>, <quic_mohamull@quicinc.com>,
	<quic_hbandi@quicinc.com>, <quic_anubhavg@quicinc.com>
Subject: Re: [PATCH] Bluetooth: qca: generalise device address check
Date: Thu, 2 May 2024 22:33:34 +0530	[thread overview]
Message-ID: <1feddcbc-205d-4c9b-bde2-7a2daace71a9@quicinc.com> (raw)
In-Reply-To: <ZjNm3OnJ1fdHctaZ@hovoldconsulting.com>

Hi Johan,

On 5/2/2024 3:41 PM, Johan Hovold wrote:
> On Thu, May 02, 2024 at 12:35:19PM +0530, Janaki Ramaiah Thota wrote:
>> On 4/30/2024 6:37 PM, Johan Hovold wrote:
> 
>>> But here we disagree. A non-unique address is not a valid one as it will
>>> cause collisions if you have more than one such controller.
>>>
>>> I understand that this may be convenient/good enough for developers in
>>> some cases, but this can hurt end users that do not realise why things
>>> break.
>>>
>>> And a developer can always configure an address manually or patch the
>>> driver as needed for internal use.
>>>
>>> Are there any other reasons that makes you want to keep the option to
>>> configure the device address through NVM files? I'm assuming you're not
>>> relying on patching NVM files to provision device-specific addresses
>>> after installation on target?
> 
>> We prefer unique address to be flashed on OTP (persistent) memory of
>> BT-Chip, which is supported by almost all QC BT-chips.
> 
> Yes, that is certainly the best option for everyone.
> 
>> If someone is not able to do that/ does not prefer that, they still
>> have an option to flash unique address in firmware binary (NVM)file.
>> This does not require setting BD address from user space.
>>
>> Also until a developer flashes OTP/ keep unique BD-Address in NVM,
>> he should be able to run most of the use cases from Device, that's
>> why we want to make it as configured.
> 
> Ok, but a developer can still do this since they can patch the driver to
> disable the check temporarily or, alternatively, just update the
> devicetree with a valid unique address.
> 
>> In our opinion this provides best Out of box experience.
> 

If a developer has to patch a code/update device-tree, that is not
a "out of box" experience. By "out of box" we meant, things should
work without much changes required.

> You can also look into improving support in user space (e.g. bluez) for
> providing a valid unique address in a simple text-based configuration
> file.
> 

We don't think putting a must-have dependency in user space is the
right thing to do, especially when we own a code in kernel space.

> That would be useful for all Linux users and not require having access
> to Qualcomm specific tools to update the NVM configuration file (which
> could also be in a read-only file system, e.g. on Android).
> 

Having a non-unique valid address allows a developer to handle all
scenarios where he/she is dealing with DUT + commercial device and
in such case, default BD-Address from nvm file should also be okay.
Only when 2/more similar devices are in the mix, they need unique
address. In that case we are providing end developers with a NVM
utility(part of Qcom build Not open source tool)to change this
default BD-Address.

> Johan

-Janaki Ram