Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3666-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <4313bcc9-8fbf-477d-90f2-013681cdebb9@quicinc.com>
Date: Wed, 17 Apr 2024 17:38:59 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] Bluetooth: qca: Fix nullptr dereference for non-serdev
 devices
To: Johan Hovold <johan@kernel.org>
CC: <luiz.dentz@gmail.com>, <marcel@holtmann.org>,
        <linux-bluetooth@vger.kernel.org>, <johan+linaro@kernel.org>
References: <1713325792-17181-1-git-send-email-quic_zijuhu@quicinc.com>
 <Zh9saGbOl-fXP6SD@hovoldconsulting.com>
 <5e4e62c8-d17f-45fd-9212-088eb026dc7b@quicinc.com>
 <Zh91zq13nZvH3-Yj@hovoldconsulting.com>
 <4dac0caa-629d-4b4d-ad54-f6ab15268524@quicinc.com>
 <Zh-JNzlskY6ZvsSm@hovoldconsulting.com>
Content-Language: en-US
From: quic_zijuhu <quic_zijuhu@quicinc.com>
In-Reply-To: <Zh-JNzlskY6ZvsSm@hovoldconsulting.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 4/17/2024 4:32 PM, Johan Hovold wrote:
> Again, make sure wrap you replies at 72 cols and trim unnecessary
> context:
> 
>   https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
> 
> On Wed, Apr 17, 2024 at 03:32:51PM +0800, quic_zijuhu wrote:
>> On 4/17/2024 3:10 PM, Johan Hovold wrote:
>>> On Wed, Apr 17, 2024 at 02:51:38PM +0800, quic_zijuhu wrote:
> 
>>>> i have submitted below patches to add supports for all other
>>>> non-serdev controllers.
>>>
>>>> https://patchwork.kernel.org/project/bluetooth/list/?series=844120
>>>
>>> Ok, you need it for some future changes, but I'm afraid that adding new
>>> random vendor specific ioctls like you do in that is series is a no-go.
> 
>> it is a generic ioctl, for QCA, it is used to specific soc_type. it
>> maybe be used by other vendor driver to set user specified info.
> 
> In it's current form it's a vendor specific hack that is never going to
> make it upstream.
>
no, i don't think so.

1)
ioctl()'s designed purpose is to complete such non-standard config.

2) present ioctl HCIUARTGETPROTO which is not exported is used to specify which vendor protocol is used
 is it a a vendor specific hack?

3)
hci_ldisc driver don't touch user specified settings and pass it into vendor driver directly
does it has any problem?

4)
for tool btattach. it does NOT get any board config info from DT|ACPI compared with
formal BT driver. so i introduce a new ioctl to supplement such info when possible
to make btattach work.

 
> For a start, you don't even make sure that the types becomes part of the
> ABI, which means that passing, say, type 5 can mean different things
> depending on the kernel version.
>it is specified by user and ONLY be parsed by vendor device driver.
it is user's responsibility to specify the right value. 
so i also don't check and care about its value and it don't need to change any code
for further added any new soc_types.

moreover, tool attach is mainly used before the final product phase. namely, its is mainly used
by developer and customer's evaluation.
 
> Can't you just retrieve the device type from the device itself? If it's
> already powered, you should not need to know this beforehand1) it is the simplest and lowest risk fix
2) different soc_types have different responses when read its IDS as shown by qca_read_soc_version().
3) the way you mentioned will involve many changes and it also means high risks for many current soc types.
> Johan