| Message ID | 1635837177-1341-1-git-send-email-zijuhu@codeaurora.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1,1/3] serdev: Add interface serdev_device_ioctl | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
On Tue, Nov 02, 2021 at 03:12:57PM +0800, Zijun Hu wrote: > From: Zijun Hu <quic_zijuhu@quicinc.com> > > Add support for MAPLE integrated within SOC, it is mounted on > a virtual tty port and powered on/off via relevant IOCTL, neither > IBS nor RAMPATCH downloading is not required. > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > --- > drivers/bluetooth/btqca.c | 13 ++++++++++++- > drivers/bluetooth/btqca.h | 13 +++++++++++++ > drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c > index be04d74037d2..b83d2ecefe5d 100644 > --- a/drivers/bluetooth/btqca.c > +++ b/drivers/bluetooth/btqca.c > @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, > BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); > BT_DBG("Length\t\t : %d bytes", length); > > + if (qca_is_maple(soc_type)) > + break; > idx = 0; > data = tlv->data; > while (idx < length) { > @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); > > /* Download rampatch file */ > + if (qca_is_maple(soc_type)) > + goto download_nvm; > + > config.type = TLV_TYPE_PATCH; > if (qca_is_wcn399x(soc_type)) { > snprintf(config.fwname, sizeof(config.fwname), > @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > /* Give the controller some time to get ready to receive the NVM */ > msleep(10); > > +download_nvm: > /* Download NVM configuration */ > config.type = TLV_TYPE_NVM; > if (firmware_name) > @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > else if (soc_type == QCA_QCA6390) > snprintf(config.fwname, sizeof(config.fwname), > "qca/htnv%02x.bin", rom_ver); > + else if (qca_is_maple(soc_type)) > + snprintf(config.fwname, sizeof(config.fwname), > + "qca/mpnv%02x.bin", rom_ver); > else if (soc_type == QCA_WCN6750) > snprintf(config.fwname, sizeof(config.fwname), > "qca/msnv%02x.bin", rom_ver); > @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); > return err; > } > + if (qca_is_maple(soc_type)) > + msleep(MAPLE_NVM_READY_DELAY_MS); > > if (soc_type >= QCA_WCN3991) { > err = qca_disable_soc_logging(hdev); > @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > return err; > } > > - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { > + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { > /* get fw build info */ > err = qca_read_fw_build_info(hdev); > if (err < 0) > diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h > index 30afa7703afd..0a5a7d1daa71 100644 > --- a/drivers/bluetooth/btqca.h > +++ b/drivers/bluetooth/btqca.h > @@ -46,6 +46,8 @@ > > #define QCA_FW_BUILD_VER_LEN 255 > > +#define MAPLE_NVM_READY_DELAY_MS 1500 > +#define MAPLE_POWER_CONTROL_DELAY_MS 50 > > enum qca_baudrate { > QCA_BAUDRATE_115200 = 0, > @@ -145,6 +147,7 @@ enum qca_btsoc_type { > QCA_WCN3991, > QCA_QCA6390, > QCA_WCN6750, > + QCA_MAPLE, > }; > > #if IS_ENABLED(CONFIG_BT_QCA) > @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > return soc_type == QCA_WCN6750; > } > > +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > +{ > + return soc_type == QCA_MAPLE; > +} > + > #else > > static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) > @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > return false; > } > > +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > +{ > + return false; > +} > + > static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) > { > return -EOPNOTSUPP; > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index dd768a8ed7cb..f1d9670719c4 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -70,6 +70,10 @@ > #define QCA_CRASHBYTE_PACKET_LEN 1096 > #define QCA_MEMDUMP_BYTE 0xFB > > +#ifndef IOCTL_IPC_BOOT > +#define IOCTL_IPC_BOOT 0xBE > +#endif You send this command, but never use it. Where is the driver code that uses this command? And why not tabs? And why is this patch series not properly threaded so tools can pick it up and find them? And why the odd named ioctl that is different from other ones in this file? And why not just use normal power management hooks for doing things like turning on and off the hardware like all other drivers? thanks, greg k-h
Hi Greg, >> Add support for MAPLE integrated within SOC, it is mounted on >> a virtual tty port and powered on/off via relevant IOCTL, neither >> IBS nor RAMPATCH downloading is not required. >> >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >> --- >> drivers/bluetooth/btqca.c | 13 ++++++++++++- >> drivers/bluetooth/btqca.h | 13 +++++++++++++ >> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 71 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >> index be04d74037d2..b83d2ecefe5d 100644 >> --- a/drivers/bluetooth/btqca.c >> +++ b/drivers/bluetooth/btqca.c >> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >> BT_DBG("Length\t\t : %d bytes", length); >> >> + if (qca_is_maple(soc_type)) >> + break; >> idx = 0; >> data = tlv->data; >> while (idx < length) { >> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >> >> /* Download rampatch file */ >> + if (qca_is_maple(soc_type)) >> + goto download_nvm; >> + >> config.type = TLV_TYPE_PATCH; >> if (qca_is_wcn399x(soc_type)) { >> snprintf(config.fwname, sizeof(config.fwname), >> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> /* Give the controller some time to get ready to receive the NVM */ >> msleep(10); >> >> +download_nvm: >> /* Download NVM configuration */ >> config.type = TLV_TYPE_NVM; >> if (firmware_name) >> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> else if (soc_type == QCA_QCA6390) >> snprintf(config.fwname, sizeof(config.fwname), >> "qca/htnv%02x.bin", rom_ver); >> + else if (qca_is_maple(soc_type)) >> + snprintf(config.fwname, sizeof(config.fwname), >> + "qca/mpnv%02x.bin", rom_ver); >> else if (soc_type == QCA_WCN6750) >> snprintf(config.fwname, sizeof(config.fwname), >> "qca/msnv%02x.bin", rom_ver); >> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >> return err; >> } >> + if (qca_is_maple(soc_type)) >> + msleep(MAPLE_NVM_READY_DELAY_MS); >> >> if (soc_type >= QCA_WCN3991) { >> err = qca_disable_soc_logging(hdev); >> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> return err; >> } >> >> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >> /* get fw build info */ >> err = qca_read_fw_build_info(hdev); >> if (err < 0) >> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >> index 30afa7703afd..0a5a7d1daa71 100644 >> --- a/drivers/bluetooth/btqca.h >> +++ b/drivers/bluetooth/btqca.h >> @@ -46,6 +46,8 @@ >> >> #define QCA_FW_BUILD_VER_LEN 255 >> >> +#define MAPLE_NVM_READY_DELAY_MS 1500 >> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >> >> enum qca_baudrate { >> QCA_BAUDRATE_115200 = 0, >> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >> QCA_WCN3991, >> QCA_QCA6390, >> QCA_WCN6750, >> + QCA_MAPLE, >> }; >> >> #if IS_ENABLED(CONFIG_BT_QCA) >> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >> return soc_type == QCA_WCN6750; >> } >> >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >> +{ >> + return soc_type == QCA_MAPLE; >> +} >> + >> #else >> >> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >> return false; >> } >> >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >> +{ >> + return false; >> +} >> + >> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >> { >> return -EOPNOTSUPP; >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index dd768a8ed7cb..f1d9670719c4 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -70,6 +70,10 @@ >> #define QCA_CRASHBYTE_PACKET_LEN 1096 >> #define QCA_MEMDUMP_BYTE 0xFB >> >> +#ifndef IOCTL_IPC_BOOT >> +#define IOCTL_IPC_BOOT 0xBE >> +#endif > > You send this command, but never use it. Where is the driver code that > uses this command? > > And why not tabs? > > And why is this patch series not properly threaded so tools can pick it > up and find them? > > And why the odd named ioctl that is different from other ones in this > file? > > And why not just use normal power management hooks for doing things like > turning on and off the hardware like all other drivers? I am not merging this. We are not starting an IPC via an external ioctl. Regards Marcel
On 11/2/2021 3:35 PM, Greg KH wrote: > On Tue, Nov 02, 2021 at 03:12:57PM +0800, Zijun Hu wrote: >> From: Zijun Hu <quic_zijuhu@quicinc.com> >> >> Add support for MAPLE integrated within SOC, it is mounted on >> a virtual tty port and powered on/off via relevant IOCTL, neither >> IBS nor RAMPATCH downloading is not required. >> >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >> --- >> drivers/bluetooth/btqca.c | 13 ++++++++++++- >> drivers/bluetooth/btqca.h | 13 +++++++++++++ >> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 71 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >> index be04d74037d2..b83d2ecefe5d 100644 >> --- a/drivers/bluetooth/btqca.c >> +++ b/drivers/bluetooth/btqca.c >> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >> BT_DBG("Length\t\t : %d bytes", length); >> >> + if (qca_is_maple(soc_type)) >> + break; >> idx = 0; >> data = tlv->data; >> while (idx < length) { >> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >> >> /* Download rampatch file */ >> + if (qca_is_maple(soc_type)) >> + goto download_nvm; >> + >> config.type = TLV_TYPE_PATCH; >> if (qca_is_wcn399x(soc_type)) { >> snprintf(config.fwname, sizeof(config.fwname), >> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> /* Give the controller some time to get ready to receive the NVM */ >> msleep(10); >> >> +download_nvm: >> /* Download NVM configuration */ >> config.type = TLV_TYPE_NVM; >> if (firmware_name) >> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> else if (soc_type == QCA_QCA6390) >> snprintf(config.fwname, sizeof(config.fwname), >> "qca/htnv%02x.bin", rom_ver); >> + else if (qca_is_maple(soc_type)) >> + snprintf(config.fwname, sizeof(config.fwname), >> + "qca/mpnv%02x.bin", rom_ver); >> else if (soc_type == QCA_WCN6750) >> snprintf(config.fwname, sizeof(config.fwname), >> "qca/msnv%02x.bin", rom_ver); >> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >> return err; >> } >> + if (qca_is_maple(soc_type)) >> + msleep(MAPLE_NVM_READY_DELAY_MS); >> >> if (soc_type >= QCA_WCN3991) { >> err = qca_disable_soc_logging(hdev); >> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >> return err; >> } >> >> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >> /* get fw build info */ >> err = qca_read_fw_build_info(hdev); >> if (err < 0) >> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >> index 30afa7703afd..0a5a7d1daa71 100644 >> --- a/drivers/bluetooth/btqca.h >> +++ b/drivers/bluetooth/btqca.h >> @@ -46,6 +46,8 @@ >> >> #define QCA_FW_BUILD_VER_LEN 255 >> >> +#define MAPLE_NVM_READY_DELAY_MS 1500 >> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >> >> enum qca_baudrate { >> QCA_BAUDRATE_115200 = 0, >> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >> QCA_WCN3991, >> QCA_QCA6390, >> QCA_WCN6750, >> + QCA_MAPLE, >> }; >> >> #if IS_ENABLED(CONFIG_BT_QCA) >> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >> return soc_type == QCA_WCN6750; >> } >> >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >> +{ >> + return soc_type == QCA_MAPLE; >> +} >> + >> #else >> >> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >> return false; >> } >> >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >> +{ >> + return false; >> +} >> + >> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >> { >> return -EOPNOTSUPP; >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index dd768a8ed7cb..f1d9670719c4 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -70,6 +70,10 @@ >> #define QCA_CRASHBYTE_PACKET_LEN 1096 >> #define QCA_MEMDUMP_BYTE 0xFB >> >> +#ifndef IOCTL_IPC_BOOT >> +#define IOCTL_IPC_BOOT 0xBE >> +#endif > > You send this command, but never use it. Where is the driver code that > uses this command? > qca_maple_power_control() will use it. this driver depends on bt_tty kernel module https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 > And why not tabs? > > And why is this patch series not properly threaded so tools can pick it > up and find them? > > And why the odd named ioctl that is different from other ones in this > file? > that IOCTL name is defined by that module. https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 > And why not just use normal power management hooks for doing things like > turning on and off the hardware like all other drivers? > this device is special. it seems BT maintainer decides to drop this patch. so please ignore it. thanks > thanks, > > greg k-h >
On Tue, Nov 02, 2021 at 03:53:33PM +0800, Zijun Hu wrote: > > > On 11/2/2021 3:35 PM, Greg KH wrote: > > On Tue, Nov 02, 2021 at 03:12:57PM +0800, Zijun Hu wrote: > >> From: Zijun Hu <quic_zijuhu@quicinc.com> > >> > >> Add support for MAPLE integrated within SOC, it is mounted on > >> a virtual tty port and powered on/off via relevant IOCTL, neither > >> IBS nor RAMPATCH downloading is not required. > >> > >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > >> --- > >> drivers/bluetooth/btqca.c | 13 ++++++++++++- > >> drivers/bluetooth/btqca.h | 13 +++++++++++++ > >> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- > >> 3 files changed, 71 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c > >> index be04d74037d2..b83d2ecefe5d 100644 > >> --- a/drivers/bluetooth/btqca.c > >> +++ b/drivers/bluetooth/btqca.c > >> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, > >> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); > >> BT_DBG("Length\t\t : %d bytes", length); > >> > >> + if (qca_is_maple(soc_type)) > >> + break; > >> idx = 0; > >> data = tlv->data; > >> while (idx < length) { > >> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); > >> > >> /* Download rampatch file */ > >> + if (qca_is_maple(soc_type)) > >> + goto download_nvm; > >> + > >> config.type = TLV_TYPE_PATCH; > >> if (qca_is_wcn399x(soc_type)) { > >> snprintf(config.fwname, sizeof(config.fwname), > >> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >> /* Give the controller some time to get ready to receive the NVM */ > >> msleep(10); > >> > >> +download_nvm: > >> /* Download NVM configuration */ > >> config.type = TLV_TYPE_NVM; > >> if (firmware_name) > >> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >> else if (soc_type == QCA_QCA6390) > >> snprintf(config.fwname, sizeof(config.fwname), > >> "qca/htnv%02x.bin", rom_ver); > >> + else if (qca_is_maple(soc_type)) > >> + snprintf(config.fwname, sizeof(config.fwname), > >> + "qca/mpnv%02x.bin", rom_ver); > >> else if (soc_type == QCA_WCN6750) > >> snprintf(config.fwname, sizeof(config.fwname), > >> "qca/msnv%02x.bin", rom_ver); > >> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); > >> return err; > >> } > >> + if (qca_is_maple(soc_type)) > >> + msleep(MAPLE_NVM_READY_DELAY_MS); > >> > >> if (soc_type >= QCA_WCN3991) { > >> err = qca_disable_soc_logging(hdev); > >> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >> return err; > >> } > >> > >> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { > >> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { > >> /* get fw build info */ > >> err = qca_read_fw_build_info(hdev); > >> if (err < 0) > >> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h > >> index 30afa7703afd..0a5a7d1daa71 100644 > >> --- a/drivers/bluetooth/btqca.h > >> +++ b/drivers/bluetooth/btqca.h > >> @@ -46,6 +46,8 @@ > >> > >> #define QCA_FW_BUILD_VER_LEN 255 > >> > >> +#define MAPLE_NVM_READY_DELAY_MS 1500 > >> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 > >> > >> enum qca_baudrate { > >> QCA_BAUDRATE_115200 = 0, > >> @@ -145,6 +147,7 @@ enum qca_btsoc_type { > >> QCA_WCN3991, > >> QCA_QCA6390, > >> QCA_WCN6750, > >> + QCA_MAPLE, > >> }; > >> > >> #if IS_ENABLED(CONFIG_BT_QCA) > >> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > >> return soc_type == QCA_WCN6750; > >> } > >> > >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > >> +{ > >> + return soc_type == QCA_MAPLE; > >> +} > >> + > >> #else > >> > >> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) > >> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > >> return false; > >> } > >> > >> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > >> +{ > >> + return false; > >> +} > >> + > >> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) > >> { > >> return -EOPNOTSUPP; > >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > >> index dd768a8ed7cb..f1d9670719c4 100644 > >> --- a/drivers/bluetooth/hci_qca.c > >> +++ b/drivers/bluetooth/hci_qca.c > >> @@ -70,6 +70,10 @@ > >> #define QCA_CRASHBYTE_PACKET_LEN 1096 > >> #define QCA_MEMDUMP_BYTE 0xFB > >> > >> +#ifndef IOCTL_IPC_BOOT > >> +#define IOCTL_IPC_BOOT 0xBE > >> +#endif > > > > You send this command, but never use it. Where is the driver code that > > uses this command? > > > qca_maple_power_control() will use it. this driver depends on bt_tty kernel module > https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 You can not add code to the kernel that is not used by the kernel itself. That driver needs to be in the tree as well, why is it not submitted now too? > > And why not tabs? > > > > And why is this patch series not properly threaded so tools can pick it > > up and find them? > > > > And why the odd named ioctl that is different from other ones in this > > file? > > > that IOCTL name is defined by that module. > https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 Again, it needs to be in the tree. > > And why not just use normal power management hooks for doing things like > > turning on and off the hardware like all other drivers? > > > this device is special. All drivers and devices are special and unique. Just like all of them :) What is so odd about this device that it can not work with the existing infrastructure that the kernel has for all of the hundreds of thousands of other devices it supports? > it seems BT maintainer decides to drop this patch. Of course, at the very least because there is no in-kernel user, why would you accept such a patch if you were the maintainer? Please submit your driver first. thanks, greg k-h
Hi Greg, >>>> Add support for MAPLE integrated within SOC, it is mounted on >>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>> IBS nor RAMPATCH downloading is not required. >>>> >>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>> --- >>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>> index be04d74037d2..b83d2ecefe5d 100644 >>>> --- a/drivers/bluetooth/btqca.c >>>> +++ b/drivers/bluetooth/btqca.c >>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>> BT_DBG("Length\t\t : %d bytes", length); >>>> >>>> + if (qca_is_maple(soc_type)) >>>> + break; >>>> idx = 0; >>>> data = tlv->data; >>>> while (idx < length) { >>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>> >>>> /* Download rampatch file */ >>>> + if (qca_is_maple(soc_type)) >>>> + goto download_nvm; >>>> + >>>> config.type = TLV_TYPE_PATCH; >>>> if (qca_is_wcn399x(soc_type)) { >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> /* Give the controller some time to get ready to receive the NVM */ >>>> msleep(10); >>>> >>>> +download_nvm: >>>> /* Download NVM configuration */ >>>> config.type = TLV_TYPE_NVM; >>>> if (firmware_name) >>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> else if (soc_type == QCA_QCA6390) >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> "qca/htnv%02x.bin", rom_ver); >>>> + else if (qca_is_maple(soc_type)) >>>> + snprintf(config.fwname, sizeof(config.fwname), >>>> + "qca/mpnv%02x.bin", rom_ver); >>>> else if (soc_type == QCA_WCN6750) >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> "qca/msnv%02x.bin", rom_ver); >>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>> return err; >>>> } >>>> + if (qca_is_maple(soc_type)) >>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>> >>>> if (soc_type >= QCA_WCN3991) { >>>> err = qca_disable_soc_logging(hdev); >>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> return err; >>>> } >>>> >>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>> /* get fw build info */ >>>> err = qca_read_fw_build_info(hdev); >>>> if (err < 0) >>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>> --- a/drivers/bluetooth/btqca.h >>>> +++ b/drivers/bluetooth/btqca.h >>>> @@ -46,6 +46,8 @@ >>>> >>>> #define QCA_FW_BUILD_VER_LEN 255 >>>> >>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>> >>>> enum qca_baudrate { >>>> QCA_BAUDRATE_115200 = 0, >>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>> QCA_WCN3991, >>>> QCA_QCA6390, >>>> QCA_WCN6750, >>>> + QCA_MAPLE, >>>> }; >>>> >>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>> return soc_type == QCA_WCN6750; >>>> } >>>> >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>> +{ >>>> + return soc_type == QCA_MAPLE; >>>> +} >>>> + >>>> #else >>>> >>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>> return false; >>>> } >>>> >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>> +{ >>>> + return false; >>>> +} >>>> + >>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>> { >>>> return -EOPNOTSUPP; >>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>> --- a/drivers/bluetooth/hci_qca.c >>>> +++ b/drivers/bluetooth/hci_qca.c >>>> @@ -70,6 +70,10 @@ >>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>> #define QCA_MEMDUMP_BYTE 0xFB >>>> >>>> +#ifndef IOCTL_IPC_BOOT >>>> +#define IOCTL_IPC_BOOT 0xBE >>>> +#endif >>> >>> You send this command, but never use it. Where is the driver code that >>> uses this command? >>> >> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 > > You can not add code to the kernel that is not used by the kernel > itself. That driver needs to be in the tree as well, why is it not > submitted now too? > >>> And why not tabs? >>> >>> And why is this patch series not properly threaded so tools can pick it >>> up and find them? >>> >>> And why the odd named ioctl that is different from other ones in this >>> file? >>> >> that IOCTL name is defined by that module. >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 > > Again, it needs to be in the tree. > >>> And why not just use normal power management hooks for doing things like >>> turning on and off the hardware like all other drivers? >>> >> this device is special. > > All drivers and devices are special and unique. Just like all of them :) > > What is so odd about this device that it can not work with the existing > infrastructure that the kernel has for all of the hundreds of thousands > of other devices it supports? > >> it seems BT maintainer decides to drop this patch. > > Of course, at the very least because there is no in-kernel user, why > would you accept such a patch if you were the maintainer? > > Please submit your driver first. this power on via ioctl is nasty business. I am so happy that we got rid of the crucks when we finally landed serdev. Some people are working on power sequence support and alike. This needs to use proper infrastructure or extend existing infrastructure. To fit the needs. I am just 100% certain, that booting an IPC via an ioctl isn’t it. We really suffered through it in the 2.4 kernel days. The hardware needs to be described properly in device tree and the kernel needs to take all the appropriate actions if a Bluetooth device is powered on via its standard power on procedure. And that is through bluetoothd (or if you use some other Bluetooth userspace) via the exposed API from the kernel. Regards Marcel
On 11/2/2021 4:22 PM, Greg KH wrote: > On Tue, Nov 02, 2021 at 03:53:33PM +0800, Zijun Hu wrote: >> >> >> On 11/2/2021 3:35 PM, Greg KH wrote: >>> On Tue, Nov 02, 2021 at 03:12:57PM +0800, Zijun Hu wrote: >>>> From: Zijun Hu <quic_zijuhu@quicinc.com> >>>> >>>> Add support for MAPLE integrated within SOC, it is mounted on >>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>> IBS nor RAMPATCH downloading is not required. >>>> >>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>> --- >>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>> index be04d74037d2..b83d2ecefe5d 100644 >>>> --- a/drivers/bluetooth/btqca.c >>>> +++ b/drivers/bluetooth/btqca.c >>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>> BT_DBG("Length\t\t : %d bytes", length); >>>> >>>> + if (qca_is_maple(soc_type)) >>>> + break; >>>> idx = 0; >>>> data = tlv->data; >>>> while (idx < length) { >>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>> >>>> /* Download rampatch file */ >>>> + if (qca_is_maple(soc_type)) >>>> + goto download_nvm; >>>> + >>>> config.type = TLV_TYPE_PATCH; >>>> if (qca_is_wcn399x(soc_type)) { >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> /* Give the controller some time to get ready to receive the NVM */ >>>> msleep(10); >>>> >>>> +download_nvm: >>>> /* Download NVM configuration */ >>>> config.type = TLV_TYPE_NVM; >>>> if (firmware_name) >>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> else if (soc_type == QCA_QCA6390) >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> "qca/htnv%02x.bin", rom_ver); >>>> + else if (qca_is_maple(soc_type)) >>>> + snprintf(config.fwname, sizeof(config.fwname), >>>> + "qca/mpnv%02x.bin", rom_ver); >>>> else if (soc_type == QCA_WCN6750) >>>> snprintf(config.fwname, sizeof(config.fwname), >>>> "qca/msnv%02x.bin", rom_ver); >>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>> return err; >>>> } >>>> + if (qca_is_maple(soc_type)) >>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>> >>>> if (soc_type >= QCA_WCN3991) { >>>> err = qca_disable_soc_logging(hdev); >>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>> return err; >>>> } >>>> >>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>> /* get fw build info */ >>>> err = qca_read_fw_build_info(hdev); >>>> if (err < 0) >>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>> --- a/drivers/bluetooth/btqca.h >>>> +++ b/drivers/bluetooth/btqca.h >>>> @@ -46,6 +46,8 @@ >>>> >>>> #define QCA_FW_BUILD_VER_LEN 255 >>>> >>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>> >>>> enum qca_baudrate { >>>> QCA_BAUDRATE_115200 = 0, >>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>> QCA_WCN3991, >>>> QCA_QCA6390, >>>> QCA_WCN6750, >>>> + QCA_MAPLE, >>>> }; >>>> >>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>> return soc_type == QCA_WCN6750; >>>> } >>>> >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>> +{ >>>> + return soc_type == QCA_MAPLE; >>>> +} >>>> + >>>> #else >>>> >>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>> return false; >>>> } >>>> >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>> +{ >>>> + return false; >>>> +} >>>> + >>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>> { >>>> return -EOPNOTSUPP; >>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>> --- a/drivers/bluetooth/hci_qca.c >>>> +++ b/drivers/bluetooth/hci_qca.c >>>> @@ -70,6 +70,10 @@ >>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>> #define QCA_MEMDUMP_BYTE 0xFB >>>> >>>> +#ifndef IOCTL_IPC_BOOT >>>> +#define IOCTL_IPC_BOOT 0xBE >>>> +#endif >>> >>> You send this command, but never use it. Where is the driver code that >>> uses this command? >>> >> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 > > You can not add code to the kernel that is not used by the kernel > itself. That driver needs to be in the tree as well, why is it not > submitted now too? > the bt_tty driver module is not developed and maintained by me. bluetooth driver code of linux-ipq-5.4 stopped update at Sep 15 2019. many relevant changes need to be picked up from bluetooth-next if apply this patch int to linux-ipq-5.4 >>> And why not tabs? >>> >>> And why is this patch series not properly threaded so tools can pick it >>> up and find them? >>> >>> And why the odd named ioctl that is different from other ones in this >>> file? >>> >> that IOCTL name is defined by that module. >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 > > Again, it needs to be in the tree. > make sense, i will try to submit this change to linux-ipq-5.4 >>> And why not just use normal power management hooks for doing things like >>> turning on and off the hardware like all other drivers? >>> >> this device is special. > > All drivers and devices are special and unique. Just like all of them :) > > What is so odd about this device that it can not work with the existing > infrastructure that the kernel has for all of the hundreds of thousands > of other devices it supports? > UART is a standard BT H/W interface. so this change depends on bt_tty which emulate a tty port. so this change use it. >> it seems BT maintainer decides to drop this patch. > > Of course, at the very least because there is no in-kernel user, why > would you accept such a patch if you were the maintainer? > > Please submit your driver first. > okay, make sense. thank you > thanks, > > greg k-h >
On 11/2/2021 4:38 PM, Marcel Holtmann wrote: > Hi Greg, > >>>>> Add support for MAPLE integrated within SOC, it is mounted on >>>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>>> IBS nor RAMPATCH downloading is not required. >>>>> >>>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>>> --- >>>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>>> index be04d74037d2..b83d2ecefe5d 100644 >>>>> --- a/drivers/bluetooth/btqca.c >>>>> +++ b/drivers/bluetooth/btqca.c >>>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>>> BT_DBG("Length\t\t : %d bytes", length); >>>>> >>>>> + if (qca_is_maple(soc_type)) >>>>> + break; >>>>> idx = 0; >>>>> data = tlv->data; >>>>> while (idx < length) { >>>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>>> >>>>> /* Download rampatch file */ >>>>> + if (qca_is_maple(soc_type)) >>>>> + goto download_nvm; >>>>> + >>>>> config.type = TLV_TYPE_PATCH; >>>>> if (qca_is_wcn399x(soc_type)) { >>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>> /* Give the controller some time to get ready to receive the NVM */ >>>>> msleep(10); >>>>> >>>>> +download_nvm: >>>>> /* Download NVM configuration */ >>>>> config.type = TLV_TYPE_NVM; >>>>> if (firmware_name) >>>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>> else if (soc_type == QCA_QCA6390) >>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>> "qca/htnv%02x.bin", rom_ver); >>>>> + else if (qca_is_maple(soc_type)) >>>>> + snprintf(config.fwname, sizeof(config.fwname), >>>>> + "qca/mpnv%02x.bin", rom_ver); >>>>> else if (soc_type == QCA_WCN6750) >>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>> "qca/msnv%02x.bin", rom_ver); >>>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>>> return err; >>>>> } >>>>> + if (qca_is_maple(soc_type)) >>>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>>> >>>>> if (soc_type >= QCA_WCN3991) { >>>>> err = qca_disable_soc_logging(hdev); >>>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>> return err; >>>>> } >>>>> >>>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>>> /* get fw build info */ >>>>> err = qca_read_fw_build_info(hdev); >>>>> if (err < 0) >>>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>>> --- a/drivers/bluetooth/btqca.h >>>>> +++ b/drivers/bluetooth/btqca.h >>>>> @@ -46,6 +46,8 @@ >>>>> >>>>> #define QCA_FW_BUILD_VER_LEN 255 >>>>> >>>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>>> >>>>> enum qca_baudrate { >>>>> QCA_BAUDRATE_115200 = 0, >>>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>>> QCA_WCN3991, >>>>> QCA_QCA6390, >>>>> QCA_WCN6750, >>>>> + QCA_MAPLE, >>>>> }; >>>>> >>>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>> return soc_type == QCA_WCN6750; >>>>> } >>>>> >>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>> +{ >>>>> + return soc_type == QCA_MAPLE; >>>>> +} >>>>> + >>>>> #else >>>>> >>>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>> return false; >>>>> } >>>>> >>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>> +{ >>>>> + return false; >>>>> +} >>>>> + >>>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>>> { >>>>> return -EOPNOTSUPP; >>>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>>> --- a/drivers/bluetooth/hci_qca.c >>>>> +++ b/drivers/bluetooth/hci_qca.c >>>>> @@ -70,6 +70,10 @@ >>>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>>> #define QCA_MEMDUMP_BYTE 0xFB >>>>> >>>>> +#ifndef IOCTL_IPC_BOOT >>>>> +#define IOCTL_IPC_BOOT 0xBE >>>>> +#endif >>>> >>>> You send this command, but never use it. Where is the driver code that >>>> uses this command? >>>> >>> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 >> >> You can not add code to the kernel that is not used by the kernel >> itself. That driver needs to be in the tree as well, why is it not >> submitted now too? >> >>>> And why not tabs? >>>> >>>> And why is this patch series not properly threaded so tools can pick it >>>> up and find them? >>>> >>>> And why the odd named ioctl that is different from other ones in this >>>> file? >>>> >>> that IOCTL name is defined by that module. >>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 >> >> Again, it needs to be in the tree. >> >>>> And why not just use normal power management hooks for doing things like >>>> turning on and off the hardware like all other drivers? >>>> >>> this device is special. >> >> All drivers and devices are special and unique. Just like all of them :) >> >> What is so odd about this device that it can not work with the existing >> infrastructure that the kernel has for all of the hundreds of thousands >> of other devices it supports? >> >>> it seems BT maintainer decides to drop this patch. >> >> Of course, at the very least because there is no in-kernel user, why >> would you accept such a patch if you were the maintainer? >> >> Please submit your driver first. > > this power on via ioctl is nasty business. I am so happy that we got rid of > the crucks when we finally landed serdev. > > Some people are working on power sequence support and alike. This needs to > use proper infrastructure or extend existing infrastructure. To fit the > needs. > > I am just 100% certain, that booting an IPC via an ioctl isn’t it. We > really suffered through it in the 2.4 kernel days. The hardware needs to > be described properly in device tree and the kernel needs to take all > the appropriate actions if a Bluetooth device is powered on via its > standard power on procedure. And that is through bluetoothd (or if you > use some other Bluetooth userspace) via the exposed API from the kernel. > thank you. the IOCTL purpose is to bootup the special bluetooth controller. i have verified this change. i will submit this change to linux-ipq-5.4 firstly even if need to pick up many changes firstly. > Regards > > Marcel >
Hi Zijun, >>>>>> Add support for MAPLE integrated within SOC, it is mounted on >>>>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>>>> IBS nor RAMPATCH downloading is not required. >>>>>> >>>>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>>>> --- >>>>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>>>> index be04d74037d2..b83d2ecefe5d 100644 >>>>>> --- a/drivers/bluetooth/btqca.c >>>>>> +++ b/drivers/bluetooth/btqca.c >>>>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>>>> BT_DBG("Length\t\t : %d bytes", length); >>>>>> >>>>>> + if (qca_is_maple(soc_type)) >>>>>> + break; >>>>>> idx = 0; >>>>>> data = tlv->data; >>>>>> while (idx < length) { >>>>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>>>> >>>>>> /* Download rampatch file */ >>>>>> + if (qca_is_maple(soc_type)) >>>>>> + goto download_nvm; >>>>>> + >>>>>> config.type = TLV_TYPE_PATCH; >>>>>> if (qca_is_wcn399x(soc_type)) { >>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>> /* Give the controller some time to get ready to receive the NVM */ >>>>>> msleep(10); >>>>>> >>>>>> +download_nvm: >>>>>> /* Download NVM configuration */ >>>>>> config.type = TLV_TYPE_NVM; >>>>>> if (firmware_name) >>>>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>> else if (soc_type == QCA_QCA6390) >>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>> "qca/htnv%02x.bin", rom_ver); >>>>>> + else if (qca_is_maple(soc_type)) >>>>>> + snprintf(config.fwname, sizeof(config.fwname), >>>>>> + "qca/mpnv%02x.bin", rom_ver); >>>>>> else if (soc_type == QCA_WCN6750) >>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>> "qca/msnv%02x.bin", rom_ver); >>>>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>>>> return err; >>>>>> } >>>>>> + if (qca_is_maple(soc_type)) >>>>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>>>> >>>>>> if (soc_type >= QCA_WCN3991) { >>>>>> err = qca_disable_soc_logging(hdev); >>>>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>> return err; >>>>>> } >>>>>> >>>>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>>>> /* get fw build info */ >>>>>> err = qca_read_fw_build_info(hdev); >>>>>> if (err < 0) >>>>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>>>> --- a/drivers/bluetooth/btqca.h >>>>>> +++ b/drivers/bluetooth/btqca.h >>>>>> @@ -46,6 +46,8 @@ >>>>>> >>>>>> #define QCA_FW_BUILD_VER_LEN 255 >>>>>> >>>>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>>>> >>>>>> enum qca_baudrate { >>>>>> QCA_BAUDRATE_115200 = 0, >>>>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>>>> QCA_WCN3991, >>>>>> QCA_QCA6390, >>>>>> QCA_WCN6750, >>>>>> + QCA_MAPLE, >>>>>> }; >>>>>> >>>>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>> return soc_type == QCA_WCN6750; >>>>>> } >>>>>> >>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>> +{ >>>>>> + return soc_type == QCA_MAPLE; >>>>>> +} >>>>>> + >>>>>> #else >>>>>> >>>>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>> return false; >>>>>> } >>>>>> >>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>> +{ >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>>>> { >>>>>> return -EOPNOTSUPP; >>>>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>>>> --- a/drivers/bluetooth/hci_qca.c >>>>>> +++ b/drivers/bluetooth/hci_qca.c >>>>>> @@ -70,6 +70,10 @@ >>>>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>>>> #define QCA_MEMDUMP_BYTE 0xFB >>>>>> >>>>>> +#ifndef IOCTL_IPC_BOOT >>>>>> +#define IOCTL_IPC_BOOT 0xBE >>>>>> +#endif >>>>> >>>>> You send this command, but never use it. Where is the driver code that >>>>> uses this command? >>>>> >>>> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 >>> >>> You can not add code to the kernel that is not used by the kernel >>> itself. That driver needs to be in the tree as well, why is it not >>> submitted now too? >>> >>>>> And why not tabs? >>>>> >>>>> And why is this patch series not properly threaded so tools can pick it >>>>> up and find them? >>>>> >>>>> And why the odd named ioctl that is different from other ones in this >>>>> file? >>>>> >>>> that IOCTL name is defined by that module. >>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 >>> >>> Again, it needs to be in the tree. >>> >>>>> And why not just use normal power management hooks for doing things like >>>>> turning on and off the hardware like all other drivers? >>>>> >>>> this device is special. >>> >>> All drivers and devices are special and unique. Just like all of them :) >>> >>> What is so odd about this device that it can not work with the existing >>> infrastructure that the kernel has for all of the hundreds of thousands >>> of other devices it supports? >>> >>>> it seems BT maintainer decides to drop this patch. >>> >>> Of course, at the very least because there is no in-kernel user, why >>> would you accept such a patch if you were the maintainer? >>> >>> Please submit your driver first. >> >> this power on via ioctl is nasty business. I am so happy that we got rid of >> the crucks when we finally landed serdev. >> >> Some people are working on power sequence support and alike. This needs to >> use proper infrastructure or extend existing infrastructure. To fit the >> needs. >> >> I am just 100% certain, that booting an IPC via an ioctl isn’t it. We >> really suffered through it in the 2.4 kernel days. The hardware needs to >> be described properly in device tree and the kernel needs to take all >> the appropriate actions if a Bluetooth device is powered on via its >> standard power on procedure. And that is through bluetoothd (or if you >> use some other Bluetooth userspace) via the exposed API from the kernel. >> > thank you. the IOCTL purpose is to bootup the special bluetooth controller. > i have verified this change. > i will submit this change to linux-ipq-5.4 firstly even if need to pick up many changes firstly. I am not sure this is fully understood yet. Do _not_ use an ioctl to boot the Bluetooth controller. Power on/off of Bluetooth hardware happens via the standard interface used by bluetoothd. The Bluetooth transport driver (in your case hci_qca) has to do everything needed when a) serdev->probe is called and b) hdev->open is called. Any other path to power your hardware is (bluntly put) wrong. Regards Marcel
On 11/2/2021 5:22 PM, Marcel Holtmann wrote: > Hi Zijun, > >>>>>>> Add support for MAPLE integrated within SOC, it is mounted on >>>>>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>>>>> IBS nor RAMPATCH downloading is not required. >>>>>>> >>>>>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>>>>> --- >>>>>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>>>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>>>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>>>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>>>>> index be04d74037d2..b83d2ecefe5d 100644 >>>>>>> --- a/drivers/bluetooth/btqca.c >>>>>>> +++ b/drivers/bluetooth/btqca.c >>>>>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>>>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>>>>> BT_DBG("Length\t\t : %d bytes", length); >>>>>>> >>>>>>> + if (qca_is_maple(soc_type)) >>>>>>> + break; >>>>>>> idx = 0; >>>>>>> data = tlv->data; >>>>>>> while (idx < length) { >>>>>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>>>>> >>>>>>> /* Download rampatch file */ >>>>>>> + if (qca_is_maple(soc_type)) >>>>>>> + goto download_nvm; >>>>>>> + >>>>>>> config.type = TLV_TYPE_PATCH; >>>>>>> if (qca_is_wcn399x(soc_type)) { >>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>> /* Give the controller some time to get ready to receive the NVM */ >>>>>>> msleep(10); >>>>>>> >>>>>>> +download_nvm: >>>>>>> /* Download NVM configuration */ >>>>>>> config.type = TLV_TYPE_NVM; >>>>>>> if (firmware_name) >>>>>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>> else if (soc_type == QCA_QCA6390) >>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>> "qca/htnv%02x.bin", rom_ver); >>>>>>> + else if (qca_is_maple(soc_type)) >>>>>>> + snprintf(config.fwname, sizeof(config.fwname), >>>>>>> + "qca/mpnv%02x.bin", rom_ver); >>>>>>> else if (soc_type == QCA_WCN6750) >>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>> "qca/msnv%02x.bin", rom_ver); >>>>>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>>>>> return err; >>>>>>> } >>>>>>> + if (qca_is_maple(soc_type)) >>>>>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>>>>> >>>>>>> if (soc_type >= QCA_WCN3991) { >>>>>>> err = qca_disable_soc_logging(hdev); >>>>>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>> return err; >>>>>>> } >>>>>>> >>>>>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>>>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>>>>> /* get fw build info */ >>>>>>> err = qca_read_fw_build_info(hdev); >>>>>>> if (err < 0) >>>>>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>>>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>>>>> --- a/drivers/bluetooth/btqca.h >>>>>>> +++ b/drivers/bluetooth/btqca.h >>>>>>> @@ -46,6 +46,8 @@ >>>>>>> >>>>>>> #define QCA_FW_BUILD_VER_LEN 255 >>>>>>> >>>>>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>>>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>>>>> >>>>>>> enum qca_baudrate { >>>>>>> QCA_BAUDRATE_115200 = 0, >>>>>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>>>>> QCA_WCN3991, >>>>>>> QCA_QCA6390, >>>>>>> QCA_WCN6750, >>>>>>> + QCA_MAPLE, >>>>>>> }; >>>>>>> >>>>>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>>>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>>> return soc_type == QCA_WCN6750; >>>>>>> } >>>>>>> >>>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>>> +{ >>>>>>> + return soc_type == QCA_MAPLE; >>>>>>> +} >>>>>>> + >>>>>>> #else >>>>>>> >>>>>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>>>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>>> return false; >>>>>>> } >>>>>>> >>>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>>> +{ >>>>>>> + return false; >>>>>>> +} >>>>>>> + >>>>>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>>>>> { >>>>>>> return -EOPNOTSUPP; >>>>>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>>>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>>>>> --- a/drivers/bluetooth/hci_qca.c >>>>>>> +++ b/drivers/bluetooth/hci_qca.c >>>>>>> @@ -70,6 +70,10 @@ >>>>>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>>>>> #define QCA_MEMDUMP_BYTE 0xFB >>>>>>> >>>>>>> +#ifndef IOCTL_IPC_BOOT >>>>>>> +#define IOCTL_IPC_BOOT 0xBE >>>>>>> +#endif >>>>>> >>>>>> You send this command, but never use it. Where is the driver code that >>>>>> uses this command? >>>>>> >>>>> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >>>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 >>>> >>>> You can not add code to the kernel that is not used by the kernel >>>> itself. That driver needs to be in the tree as well, why is it not >>>> submitted now too? >>>> >>>>>> And why not tabs? >>>>>> >>>>>> And why is this patch series not properly threaded so tools can pick it >>>>>> up and find them? >>>>>> >>>>>> And why the odd named ioctl that is different from other ones in this >>>>>> file? >>>>>> >>>>> that IOCTL name is defined by that module. >>>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 >>>> >>>> Again, it needs to be in the tree. >>>> >>>>>> And why not just use normal power management hooks for doing things like >>>>>> turning on and off the hardware like all other drivers? >>>>>> >>>>> this device is special. >>>> >>>> All drivers and devices are special and unique. Just like all of them :) >>>> >>>> What is so odd about this device that it can not work with the existing >>>> infrastructure that the kernel has for all of the hundreds of thousands >>>> of other devices it supports? >>>> >>>>> it seems BT maintainer decides to drop this patch. >>>> >>>> Of course, at the very least because there is no in-kernel user, why >>>> would you accept such a patch if you were the maintainer? >>>> >>>> Please submit your driver first. >>> >>> this power on via ioctl is nasty business. I am so happy that we got rid of >>> the crucks when we finally landed serdev. >>> >>> Some people are working on power sequence support and alike. This needs to >>> use proper infrastructure or extend existing infrastructure. To fit the >>> needs. >>> >>> I am just 100% certain, that booting an IPC via an ioctl isn’t it. We >>> really suffered through it in the 2.4 kernel days. The hardware needs to >>> be described properly in device tree and the kernel needs to take all >>> the appropriate actions if a Bluetooth device is powered on via its >>> standard power on procedure. And that is through bluetoothd (or if you >>> use some other Bluetooth userspace) via the exposed API from the kernel. >>> >> thank you. the IOCTL purpose is to bootup the special bluetooth controller. >> i have verified this change. >> i will submit this change to linux-ipq-5.4 firstly even if need to pick up many changes firstly. > > I am not sure this is fully understood yet. Do _not_ use an ioctl to boot the Bluetooth > controller. Power on/off of Bluetooth hardware happens via the standard interface used > by bluetoothd. The Bluetooth transport driver (in your case hci_qca) has to do everything > needed when a) serdev->probe is called and b) hdev->open is called. Any other path to > power your hardware is (bluntly put) wrong. > for all the other present controllers supported by hci_qca, ALL the finial initialization job is Done driver itself(hci_qca) via hdev->setup which call qca_setup(), the initialization includes power control, clock control, driver BT enable pin, download F/W. > Regards > > Marcel >
Hi Zijun, >>>>>>>> Add support for MAPLE integrated within SOC, it is mounted on >>>>>>>> a virtual tty port and powered on/off via relevant IOCTL, neither >>>>>>>> IBS nor RAMPATCH downloading is not required. >>>>>>>> >>>>>>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> >>>>>>>> --- >>>>>>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- >>>>>>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ >>>>>>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- >>>>>>>> 3 files changed, 71 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >>>>>>>> index be04d74037d2..b83d2ecefe5d 100644 >>>>>>>> --- a/drivers/bluetooth/btqca.c >>>>>>>> +++ b/drivers/bluetooth/btqca.c >>>>>>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, >>>>>>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); >>>>>>>> BT_DBG("Length\t\t : %d bytes", length); >>>>>>>> >>>>>>>> + if (qca_is_maple(soc_type)) >>>>>>>> + break; >>>>>>>> idx = 0; >>>>>>>> data = tlv->data; >>>>>>>> while (idx < length) { >>>>>>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); >>>>>>>> >>>>>>>> /* Download rampatch file */ >>>>>>>> + if (qca_is_maple(soc_type)) >>>>>>>> + goto download_nvm; >>>>>>>> + >>>>>>>> config.type = TLV_TYPE_PATCH; >>>>>>>> if (qca_is_wcn399x(soc_type)) { >>>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>>> /* Give the controller some time to get ready to receive the NVM */ >>>>>>>> msleep(10); >>>>>>>> >>>>>>>> +download_nvm: >>>>>>>> /* Download NVM configuration */ >>>>>>>> config.type = TLV_TYPE_NVM; >>>>>>>> if (firmware_name) >>>>>>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>>> else if (soc_type == QCA_QCA6390) >>>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>>> "qca/htnv%02x.bin", rom_ver); >>>>>>>> + else if (qca_is_maple(soc_type)) >>>>>>>> + snprintf(config.fwname, sizeof(config.fwname), >>>>>>>> + "qca/mpnv%02x.bin", rom_ver); >>>>>>>> else if (soc_type == QCA_WCN6750) >>>>>>>> snprintf(config.fwname, sizeof(config.fwname), >>>>>>>> "qca/msnv%02x.bin", rom_ver); >>>>>>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); >>>>>>>> return err; >>>>>>>> } >>>>>>>> + if (qca_is_maple(soc_type)) >>>>>>>> + msleep(MAPLE_NVM_READY_DELAY_MS); >>>>>>>> >>>>>>>> if (soc_type >= QCA_WCN3991) { >>>>>>>> err = qca_disable_soc_logging(hdev); >>>>>>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, >>>>>>>> return err; >>>>>>>> } >>>>>>>> >>>>>>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { >>>>>>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { >>>>>>>> /* get fw build info */ >>>>>>>> err = qca_read_fw_build_info(hdev); >>>>>>>> if (err < 0) >>>>>>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >>>>>>>> index 30afa7703afd..0a5a7d1daa71 100644 >>>>>>>> --- a/drivers/bluetooth/btqca.h >>>>>>>> +++ b/drivers/bluetooth/btqca.h >>>>>>>> @@ -46,6 +46,8 @@ >>>>>>>> >>>>>>>> #define QCA_FW_BUILD_VER_LEN 255 >>>>>>>> >>>>>>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 >>>>>>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 >>>>>>>> >>>>>>>> enum qca_baudrate { >>>>>>>> QCA_BAUDRATE_115200 = 0, >>>>>>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { >>>>>>>> QCA_WCN3991, >>>>>>>> QCA_QCA6390, >>>>>>>> QCA_WCN6750, >>>>>>>> + QCA_MAPLE, >>>>>>>> }; >>>>>>>> >>>>>>>> #if IS_ENABLED(CONFIG_BT_QCA) >>>>>>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>>>> return soc_type == QCA_WCN6750; >>>>>>>> } >>>>>>>> >>>>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>>>> +{ >>>>>>>> + return soc_type == QCA_MAPLE; >>>>>>>> +} >>>>>>>> + >>>>>>>> #else >>>>>>>> >>>>>>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) >>>>>>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) >>>>>>>> return false; >>>>>>>> } >>>>>>>> >>>>>>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) >>>>>>>> +{ >>>>>>>> + return false; >>>>>>>> +} >>>>>>>> + >>>>>>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) >>>>>>>> { >>>>>>>> return -EOPNOTSUPP; >>>>>>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>>>>>> index dd768a8ed7cb..f1d9670719c4 100644 >>>>>>>> --- a/drivers/bluetooth/hci_qca.c >>>>>>>> +++ b/drivers/bluetooth/hci_qca.c >>>>>>>> @@ -70,6 +70,10 @@ >>>>>>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 >>>>>>>> #define QCA_MEMDUMP_BYTE 0xFB >>>>>>>> >>>>>>>> +#ifndef IOCTL_IPC_BOOT >>>>>>>> +#define IOCTL_IPC_BOOT 0xBE >>>>>>>> +#endif >>>>>>> >>>>>>> You send this command, but never use it. Where is the driver code that >>>>>>> uses this command? >>>>>>> >>>>>> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module >>>>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 >>>>> >>>>> You can not add code to the kernel that is not used by the kernel >>>>> itself. That driver needs to be in the tree as well, why is it not >>>>> submitted now too? >>>>> >>>>>>> And why not tabs? >>>>>>> >>>>>>> And why is this patch series not properly threaded so tools can pick it >>>>>>> up and find them? >>>>>>> >>>>>>> And why the odd named ioctl that is different from other ones in this >>>>>>> file? >>>>>>> >>>>>> that IOCTL name is defined by that module. >>>>>> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 >>>>> >>>>> Again, it needs to be in the tree. >>>>> >>>>>>> And why not just use normal power management hooks for doing things like >>>>>>> turning on and off the hardware like all other drivers? >>>>>>> >>>>>> this device is special. >>>>> >>>>> All drivers and devices are special and unique. Just like all of them :) >>>>> >>>>> What is so odd about this device that it can not work with the existing >>>>> infrastructure that the kernel has for all of the hundreds of thousands >>>>> of other devices it supports? >>>>> >>>>>> it seems BT maintainer decides to drop this patch. >>>>> >>>>> Of course, at the very least because there is no in-kernel user, why >>>>> would you accept such a patch if you were the maintainer? >>>>> >>>>> Please submit your driver first. >>>> >>>> this power on via ioctl is nasty business. I am so happy that we got rid of >>>> the crucks when we finally landed serdev. >>>> >>>> Some people are working on power sequence support and alike. This needs to >>>> use proper infrastructure or extend existing infrastructure. To fit the >>>> needs. >>>> >>>> I am just 100% certain, that booting an IPC via an ioctl isn’t it. We >>>> really suffered through it in the 2.4 kernel days. The hardware needs to >>>> be described properly in device tree and the kernel needs to take all >>>> the appropriate actions if a Bluetooth device is powered on via its >>>> standard power on procedure. And that is through bluetoothd (or if you >>>> use some other Bluetooth userspace) via the exposed API from the kernel. >>>> >>> thank you. the IOCTL purpose is to bootup the special bluetooth controller. >>> i have verified this change. >>> i will submit this change to linux-ipq-5.4 firstly even if need to pick up many changes firstly. >> >> I am not sure this is fully understood yet. Do _not_ use an ioctl to boot the Bluetooth >> controller. Power on/off of Bluetooth hardware happens via the standard interface used >> by bluetoothd. The Bluetooth transport driver (in your case hci_qca) has to do everything >> needed when a) serdev->probe is called and b) hdev->open is called. Any other path to >> power your hardware is (bluntly put) wrong. >> > for all the other present controllers supported by hci_qca, ALL the finial initialization job is Done driver itself(hci_qca) > via hdev->setup which call qca_setup(), the initialization includes power control, clock control, driver BT enable > pin, download F/W. and what I am saying, it has to be the same for this hardware as well. You need to re-design your driver / patch. On a side note, you don’t have to add support for this hardware into hci_qca. You can write a brand new driver to support your hardware. If hci_qca is no fit, then start from scratch. However same rules apply work in the constraints of serdev->probe and hdev->open to establish the Bluetooth transport. The hdev->setup is post-transport setup and doesn’t really apply to anything powering on the controller. Regards Marcel
On Tue, Nov 02, 2021 at 04:54:23PM +0800, Zijun Hu wrote: > > > On 11/2/2021 4:22 PM, Greg KH wrote: > > On Tue, Nov 02, 2021 at 03:53:33PM +0800, Zijun Hu wrote: > >> > >> > >> On 11/2/2021 3:35 PM, Greg KH wrote: > >>> On Tue, Nov 02, 2021 at 03:12:57PM +0800, Zijun Hu wrote: > >>>> From: Zijun Hu <quic_zijuhu@quicinc.com> > >>>> > >>>> Add support for MAPLE integrated within SOC, it is mounted on > >>>> a virtual tty port and powered on/off via relevant IOCTL, neither > >>>> IBS nor RAMPATCH downloading is not required. > >>>> > >>>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > >>>> --- > >>>> drivers/bluetooth/btqca.c | 13 ++++++++++++- > >>>> drivers/bluetooth/btqca.h | 13 +++++++++++++ > >>>> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++++++++++++++++++++++++- > >>>> 3 files changed, 71 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c > >>>> index be04d74037d2..b83d2ecefe5d 100644 > >>>> --- a/drivers/bluetooth/btqca.c > >>>> +++ b/drivers/bluetooth/btqca.c > >>>> @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, > >>>> BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); > >>>> BT_DBG("Length\t\t : %d bytes", length); > >>>> > >>>> + if (qca_is_maple(soc_type)) > >>>> + break; > >>>> idx = 0; > >>>> data = tlv->data; > >>>> while (idx < length) { > >>>> @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >>>> rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); > >>>> > >>>> /* Download rampatch file */ > >>>> + if (qca_is_maple(soc_type)) > >>>> + goto download_nvm; > >>>> + > >>>> config.type = TLV_TYPE_PATCH; > >>>> if (qca_is_wcn399x(soc_type)) { > >>>> snprintf(config.fwname, sizeof(config.fwname), > >>>> @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >>>> /* Give the controller some time to get ready to receive the NVM */ > >>>> msleep(10); > >>>> > >>>> +download_nvm: > >>>> /* Download NVM configuration */ > >>>> config.type = TLV_TYPE_NVM; > >>>> if (firmware_name) > >>>> @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >>>> else if (soc_type == QCA_QCA6390) > >>>> snprintf(config.fwname, sizeof(config.fwname), > >>>> "qca/htnv%02x.bin", rom_ver); > >>>> + else if (qca_is_maple(soc_type)) > >>>> + snprintf(config.fwname, sizeof(config.fwname), > >>>> + "qca/mpnv%02x.bin", rom_ver); > >>>> else if (soc_type == QCA_WCN6750) > >>>> snprintf(config.fwname, sizeof(config.fwname), > >>>> "qca/msnv%02x.bin", rom_ver); > >>>> @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >>>> bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); > >>>> return err; > >>>> } > >>>> + if (qca_is_maple(soc_type)) > >>>> + msleep(MAPLE_NVM_READY_DELAY_MS); > >>>> > >>>> if (soc_type >= QCA_WCN3991) { > >>>> err = qca_disable_soc_logging(hdev); > >>>> @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > >>>> return err; > >>>> } > >>>> > >>>> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { > >>>> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { > >>>> /* get fw build info */ > >>>> err = qca_read_fw_build_info(hdev); > >>>> if (err < 0) > >>>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h > >>>> index 30afa7703afd..0a5a7d1daa71 100644 > >>>> --- a/drivers/bluetooth/btqca.h > >>>> +++ b/drivers/bluetooth/btqca.h > >>>> @@ -46,6 +46,8 @@ > >>>> > >>>> #define QCA_FW_BUILD_VER_LEN 255 > >>>> > >>>> +#define MAPLE_NVM_READY_DELAY_MS 1500 > >>>> +#define MAPLE_POWER_CONTROL_DELAY_MS 50 > >>>> > >>>> enum qca_baudrate { > >>>> QCA_BAUDRATE_115200 = 0, > >>>> @@ -145,6 +147,7 @@ enum qca_btsoc_type { > >>>> QCA_WCN3991, > >>>> QCA_QCA6390, > >>>> QCA_WCN6750, > >>>> + QCA_MAPLE, > >>>> }; > >>>> > >>>> #if IS_ENABLED(CONFIG_BT_QCA) > >>>> @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > >>>> return soc_type == QCA_WCN6750; > >>>> } > >>>> > >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > >>>> +{ > >>>> + return soc_type == QCA_MAPLE; > >>>> +} > >>>> + > >>>> #else > >>>> > >>>> static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) > >>>> @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) > >>>> return false; > >>>> } > >>>> > >>>> +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) > >>>> +{ > >>>> + return false; > >>>> +} > >>>> + > >>>> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) > >>>> { > >>>> return -EOPNOTSUPP; > >>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > >>>> index dd768a8ed7cb..f1d9670719c4 100644 > >>>> --- a/drivers/bluetooth/hci_qca.c > >>>> +++ b/drivers/bluetooth/hci_qca.c > >>>> @@ -70,6 +70,10 @@ > >>>> #define QCA_CRASHBYTE_PACKET_LEN 1096 > >>>> #define QCA_MEMDUMP_BYTE 0xFB > >>>> > >>>> +#ifndef IOCTL_IPC_BOOT > >>>> +#define IOCTL_IPC_BOOT 0xBE > >>>> +#endif > >>> > >>> You send this command, but never use it. Where is the driver code that > >>> uses this command? > >>> > >> qca_maple_power_control() will use it. this driver depends on bt_tty kernel module > >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/drivers/soc/qcom/bt_tty.c?h=NHSS.QSDK.11.5.0.5.r2 > > > > You can not add code to the kernel that is not used by the kernel > > itself. That driver needs to be in the tree as well, why is it not > > submitted now too? > > > the bt_tty driver module is not developed and maintained by me. > bluetooth driver code of linux-ipq-5.4 stopped update at Sep 15 2019. > many relevant changes need to be picked up from bluetooth-next if apply this patch int to linux-ipq-5.4 > > >>> And why not tabs? > >>> > >>> And why is this patch series not properly threaded so tools can pick it > >>> up and find them? > >>> > >>> And why the odd named ioctl that is different from other ones in this > >>> file? > >>> > >> that IOCTL name is defined by that module. > >> https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/tree/include/linux/bt.h?h=NHSS.QSDK.11.5.0.5.r2 > > > > Again, it needs to be in the tree. > > > make sense, i will try to submit this change to linux-ipq-5.4 "linux-ipq-5.4" has NOTHING to do with the upstream kernel development process here. Please work with the developers at your company to understand how the kernel development process works to help understand this correctly, before submitting patches again. thanks, greg k-h
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c index be04d74037d2..b83d2ecefe5d 100644 --- a/drivers/bluetooth/btqca.c +++ b/drivers/bluetooth/btqca.c @@ -255,6 +255,8 @@ static void qca_tlv_check_data(struct hci_dev *hdev, BT_DBG("TLV Type\t\t : 0x%x", type_len & 0x000000ff); BT_DBG("Length\t\t : %d bytes", length); + if (qca_is_maple(soc_type)) + break; idx = 0; data = tlv->data; while (idx < length) { @@ -552,6 +554,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f); /* Download rampatch file */ + if (qca_is_maple(soc_type)) + goto download_nvm; + config.type = TLV_TYPE_PATCH; if (qca_is_wcn399x(soc_type)) { snprintf(config.fwname, sizeof(config.fwname), @@ -580,6 +585,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, /* Give the controller some time to get ready to receive the NVM */ msleep(10); +download_nvm: /* Download NVM configuration */ config.type = TLV_TYPE_NVM; if (firmware_name) @@ -597,6 +603,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, else if (soc_type == QCA_QCA6390) snprintf(config.fwname, sizeof(config.fwname), "qca/htnv%02x.bin", rom_ver); + else if (qca_is_maple(soc_type)) + snprintf(config.fwname, sizeof(config.fwname), + "qca/mpnv%02x.bin", rom_ver); else if (soc_type == QCA_WCN6750) snprintf(config.fwname, sizeof(config.fwname), "qca/msnv%02x.bin", rom_ver); @@ -609,6 +618,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err); return err; } + if (qca_is_maple(soc_type)) + msleep(MAPLE_NVM_READY_DELAY_MS); if (soc_type >= QCA_WCN3991) { err = qca_disable_soc_logging(hdev); @@ -637,7 +648,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, return err; } - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) { + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || qca_is_maple(soc_type)) { /* get fw build info */ err = qca_read_fw_build_info(hdev); if (err < 0) diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h index 30afa7703afd..0a5a7d1daa71 100644 --- a/drivers/bluetooth/btqca.h +++ b/drivers/bluetooth/btqca.h @@ -46,6 +46,8 @@ #define QCA_FW_BUILD_VER_LEN 255 +#define MAPLE_NVM_READY_DELAY_MS 1500 +#define MAPLE_POWER_CONTROL_DELAY_MS 50 enum qca_baudrate { QCA_BAUDRATE_115200 = 0, @@ -145,6 +147,7 @@ enum qca_btsoc_type { QCA_WCN3991, QCA_QCA6390, QCA_WCN6750, + QCA_MAPLE, }; #if IS_ENABLED(CONFIG_BT_QCA) @@ -167,6 +170,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) return soc_type == QCA_WCN6750; } +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) +{ + return soc_type == QCA_MAPLE; +} + #else static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr) @@ -204,6 +212,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type) return false; } +static inline bool qca_is_maple(enum qca_btsoc_type soc_type) +{ + return false; +} + static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev) { return -EOPNOTSUPP; diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index dd768a8ed7cb..f1d9670719c4 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -70,6 +70,10 @@ #define QCA_CRASHBYTE_PACKET_LEN 1096 #define QCA_MEMDUMP_BYTE 0xFB +#ifndef IOCTL_IPC_BOOT +#define IOCTL_IPC_BOOT 0xBE +#endif + enum qca_flags { QCA_IBS_DISABLED, QCA_DROP_VENDOR_EVENT, @@ -1370,6 +1374,9 @@ static unsigned int qca_get_speed(struct hci_uart *hu, { unsigned int speed = 0; + if (qca_is_maple(qca_soc_type(hu))) + return 0; + if (speed_type == QCA_INIT_SPEED) { if (hu->init_speed) speed = hu->init_speed; @@ -1387,6 +1394,9 @@ static unsigned int qca_get_speed(struct hci_uart *hu, static int qca_check_speeds(struct hci_uart *hu) { + if (qca_is_maple(qca_soc_type(hu))) + return 0; + if (qca_is_wcn399x(qca_soc_type(hu)) || qca_is_wcn6750(qca_soc_type(hu))) { if (!qca_get_speed(hu, QCA_INIT_SPEED) && @@ -1660,6 +1670,21 @@ static int qca_regulator_init(struct hci_uart *hu) return 0; } +static int qca_maple_power_control(struct hci_uart *hu, bool on) +{ + int ret; + int power_arg = on ? 1 : 0; + + ret = serdev_device_ioctl(hu->serdev, IOCTL_IPC_BOOT, power_arg); + if (ret) + bt_dev_err(hu->hdev, "%s: power %s failure: %d\n", __func__, + on ? "ON" : "OFF", ret); + else + msleep(MAPLE_POWER_CONTROL_DELAY_MS); + + return ret; +} + static int qca_power_on(struct hci_dev *hdev) { struct hci_uart *hu = hci_get_drvdata(hdev); @@ -1677,6 +1702,8 @@ static int qca_power_on(struct hci_dev *hdev) if (qca_is_wcn399x(soc_type) || qca_is_wcn6750(soc_type)) { ret = qca_regulator_init(hu); + } else if (qca_is_maple(soc_type)) { + ret = qca_maple_power_control(hu, true); } else { qcadev = serdev_device_get_drvdata(hu->serdev); if (qcadev->bt_en) { @@ -1715,6 +1742,7 @@ static int qca_setup(struct hci_uart *hu) set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); bt_dev_info(hdev, "setting up %s", + qca_is_maple(soc_type) ? "maple" : qca_is_wcn399x(soc_type) ? "wcn399x" : (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390"); @@ -1761,7 +1789,10 @@ static int qca_setup(struct hci_uart *hu) ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver, firmware_name); if (!ret) { - clear_bit(QCA_IBS_DISABLED, &qca->flags); + if (qca_is_maple(soc_type)) + set_bit(QCA_ROM_FW, &qca->flags); + else + clear_bit(QCA_IBS_DISABLED, &qca->flags); qca_debugfs_init(hdev); hu->hdev->hw_error = qca_hw_error; hu->hdev->cmd_timeout = qca_cmd_timeout; @@ -1858,6 +1889,11 @@ static const struct qca_device_data qca_soc_data_qca6390 = { .num_vregs = 0, }; +static const struct qca_device_data qca_soc_data_maple = { + .soc_type = QCA_MAPLE, + .num_vregs = 0, +}; + static const struct qca_device_data qca_soc_data_wcn6750 = { .soc_type = QCA_WCN6750, .vregs = (struct qca_vreg []) { @@ -1912,6 +1948,8 @@ static void qca_power_shutdown(struct hci_uart *hu) sw_ctrl_state = gpiod_get_value_cansleep(qcadev->sw_ctrl); bt_dev_dbg(hu->hdev, "SW_CTRL is %d", sw_ctrl_state); } + } else if (qca_is_maple(soc_type)) { + qca_maple_power_control(hu, false); } else if (qcadev->bt_en) { gpiod_set_value_cansleep(qcadev->bt_en, 0); } @@ -2089,6 +2127,10 @@ static int qca_serdev_probe(struct serdev_device *serdev) dev_warn(&serdev->dev, "failed to acquire enable gpio\n"); power_ctrl_enabled = false; } + if (qca_is_maple(qcadev->btsoc_type)) { + dev_info(&serdev->dev, "Maple: power ctrl enabled\n"); + power_ctrl_enabled = true; + } qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); if (IS_ERR(qcadev->susclk)) { @@ -2142,6 +2184,8 @@ static void qca_serdev_remove(struct serdev_device *serdev) qca_is_wcn6750(qcadev->btsoc_type)) && power->vregs_on) qca_power_shutdown(&qcadev->serdev_hu); + else if (qca_is_maple(qcadev->btsoc_type)) + qca_power_shutdown(&qcadev->serdev_hu); else if (qcadev->susclk) clk_disable_unprepare(qcadev->susclk); @@ -2312,6 +2356,7 @@ static SIMPLE_DEV_PM_OPS(qca_pm_ops, qca_suspend, qca_resume); static const struct of_device_id qca_bluetooth_of_match[] = { { .compatible = "qcom,qca6174-bt" }, { .compatible = "qcom,qca6390-bt", .data = &qca_soc_data_qca6390}, + { .compatible = "qcom,maple-bt", .data = &qca_soc_data_maple}, { .compatible = "qcom,qca9377-bt" }, { .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990}, { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},