On Thu, May 24, 2018 at 09:30:48PM +0530, Balakrishna Godavarthi wrote:
> Some of the QCA BTSoC ROME functions, are used for different versions
> or different make of BTSoC's. Instead of duplicating the same functions
> for new chip, updating names of the functions that are used for both
> chip's to keep this generic and would help in future when we would have
> new BT SoC.
The commit message talks about 'functions', however only the name of
one function (rome_patch_ver_req()) has changed. From the changes in
log messages I assume you also intended to rename
rome_reset() and rome_download_firmware().
> Signed-off-by: Balakrishna Godavarthi <bgodavar@xxxxxxxxxxxxxx>
> ---
>
> Changes in v6:
> * initial patch
> * updated names of functions that are used for both the chips to
> keep this generic and would help in future when we would have
> new BT SoC.
>
> ---
> drivers/bluetooth/btqca.c | 14 +++++++-------
> drivers/bluetooth/btqca.h | 6 ++++++
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 8219816c54a0..5c3551239b12 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -27,7 +27,7 @@
>
> #define VERSION "0.1"
>
> -static int rome_patch_ver_req(struct hci_dev *hdev, u32 *rome_version)
> +int qca_patch_ver_req(struct hci_dev *hdev, u32 *rome_version)
The different version names are confusing. The name of the function
calls it 'patch version', the parameter 'rome_version' and the callers
vary between 'soc_version' and 'rome_version'.
It would be unfair to ask you to 'fix the world' in this patch series,
but please stick at least to a consistent naming scheme in the code
you change. Obviously feel free to squeeze in another cleanup patch :)
> static int rome_reset(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> int err;
>
> - BT_DBG("%s: ROME HCI_RESET", hdev->name);
> + bt_dev_dbg(hdev, "QCA BTSoC HCI_RESET");
>
> skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> if (IS_ERR(skb)) {
> @@ -267,7 +267,7 @@ static int rome_download_firmware(struct hci_dev *hdev,
> const u8 *segment;
> int ret, remain, i = 0;
>
> - bt_dev_info(hdev, "ROME Downloading %s", config->fwname);
> + bt_dev_info(hdev, "QCA BTSoC Downloading %s", config->fwname);
>
> ret = request_firmware(&fw, config->fwname, &hdev->dev);
> if (ret) {
> @@ -339,7 +339,7 @@ int qca_uart_setup_rome(struct hci_dev *hdev, uint8_t baudrate)
> config.user_baud_rate = baudrate;
>
> /* Get ROME version information */
'patch version'/'SoC version'/... , whatever the chosen consistent
name is.
> - err = rome_patch_ver_req(hdev, &rome_ver);
> + err = qca_patch_ver_req(hdev, &rome_ver);
rome_ver => ${consistent_name}_ver
Thanks
Matthias
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
