On 4/17/2024 5:38 PM, quic_zijuhu wrote: > 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 > BTW, it is the simplest and lowest risk fix for tool btattach
