diff mbox series

Bluetooth: MGMT: Fix Set PHY Configuration command

Message ID 20220906212421.2810556-1-luiz.dentz@gmail.com
State New
Headers show
Series Bluetooth: MGMT: Fix Set PHY Configuration command | expand

Commit Message

Luiz Augusto von Dentz Sept. 6, 2022, 9:24 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The command shall accept any subset that is considered configurable but
instead it was only accepting the exact match causing errors such as:

[mgmt]# phy
Supported phys: BR1M1SLOT BR1M3SLOT BR1M5SLOT EDR2M1SLOT EDR2M3SLOT
EDR2M5SLOT EDR3M1SLOT EDR3M3SLOT EDR3M5SLOT LE1MTX LE1MRX LE2MTX
LE2MRX LECODEDTX LECODEDRX
Configurable phys: BR1M3SLOT BR1M5SLOT EDR2M1SLOT EDR2M3SLOT EDR2M5SLOT
EDR3M1SLOT EDR3M3SLOT EDR3M5SLOT LE2MTX LE2MRX LECODEDTX LECODEDRX
Selected phys: BR1M1SLOT BR1M3SLOT BR1M5SLOT EDR2M1SLOT EDR2M3SLOT
EDR2M5SLOT EDR3M1SLOT EDR3M3SLOT EDR3M5SLOT LE1MTX LE1MRX
[mgmt]# phy BR1M3SLOT BR1M5SLOT EDR2M1SLOT EDR2M3SLOT EDR2M5SLOT
EDR3M1SLOT EDR3M3SLOT EDR3M5SLOT LE2MTX LE2MRX
Could not set PHY Configuration with status 0x0d (Invalid Parameters)

Fixes: 0314f2867fa0 ("Bluetooth: Implement Set PHY Confguration command")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/mgmt.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

kernel test robot Sept. 6, 2022, 11:30 p.m. UTC | #1 
Hi Luiz,

I love your patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master linus/master v6.0-rc4 next-20220906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-MGMT-Fix-Set-PHY-Configuration-command/20220907-052628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: m68k-allyesconfig
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3903b23ef4b5407d0c6a6c3d6cc80a8bc14393cf
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-MGMT-Fix-Set-PHY-Configuration-command/20220907-052628
        git checkout 3903b23ef4b5407d0c6a6c3d6cc80a8bc14393cf
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/bluetooth/mgmt.c: In function 'set_phy_configuration':
>> net/bluetooth/mgmt.c:4052:47: warning: variable 'supported_phys' set but not used [-Wunused-but-set-variable]
    4052 |         u32 selected_phys, configurable_phys, supported_phys;
         |                                               ^~~~~~~~~~~~~~


vim +/supported_phys +4052 net/bluetooth/mgmt.c

  4046	
  4047	static int set_phy_configuration(struct sock *sk, struct hci_dev *hdev,
  4048					 void *data, u16 len)
  4049	{
  4050		struct mgmt_cp_set_phy_configuration *cp = data;
  4051		struct mgmt_pending_cmd *cmd;
> 4052		u32 selected_phys, configurable_phys, supported_phys;
  4053		u16 pkt_type = (HCI_DH1 | HCI_DM1);
  4054		bool changed = false;
  4055		int err;
  4056	
  4057		bt_dev_dbg(hdev, "sock %p", sk);
  4058	
  4059		configurable_phys = get_configurable_phys(hdev);
  4060		supported_phys = get_supported_phys(hdev);
  4061		selected_phys = __le32_to_cpu(cp->selected_phys);
  4062	
  4063		if (selected_phys & ~configurable_phys)
  4064			return mgmt_cmd_status(sk, hdev->id,
  4065					       MGMT_OP_SET_PHY_CONFIGURATION,
  4066					       MGMT_STATUS_INVALID_PARAMS);
  4067	
  4068		if (selected_phys == get_selected_phys(hdev))
  4069			return mgmt_cmd_complete(sk, hdev->id,
  4070						 MGMT_OP_SET_PHY_CONFIGURATION,
  4071						 0, NULL, 0);
  4072	
  4073		hci_dev_lock(hdev);
  4074	
  4075		if (!hdev_is_powered(hdev)) {
  4076			err = mgmt_cmd_status(sk, hdev->id,
  4077					      MGMT_OP_SET_PHY_CONFIGURATION,
  4078					      MGMT_STATUS_REJECTED);
  4079			goto unlock;
  4080		}
  4081	
  4082		if (pending_find(MGMT_OP_SET_PHY_CONFIGURATION, hdev)) {
  4083			err = mgmt_cmd_status(sk, hdev->id,
  4084					      MGMT_OP_SET_PHY_CONFIGURATION,
  4085					      MGMT_STATUS_BUSY);
  4086			goto unlock;
  4087		}
  4088	
  4089		if (selected_phys & MGMT_PHY_BR_1M_3SLOT)
  4090			pkt_type |= (HCI_DH3 | HCI_DM3);
  4091		else
  4092			pkt_type &= ~(HCI_DH3 | HCI_DM3);
  4093	
  4094		if (selected_phys & MGMT_PHY_BR_1M_5SLOT)
  4095			pkt_type |= (HCI_DH5 | HCI_DM5);
  4096		else
  4097			pkt_type &= ~(HCI_DH5 | HCI_DM5);
  4098	
  4099		if (selected_phys & MGMT_PHY_EDR_2M_1SLOT)
  4100			pkt_type &= ~HCI_2DH1;
  4101		else
  4102			pkt_type |= HCI_2DH1;
  4103	
  4104		if (selected_phys & MGMT_PHY_EDR_2M_3SLOT)
  4105			pkt_type &= ~HCI_2DH3;
  4106		else
  4107			pkt_type |= HCI_2DH3;
  4108	
  4109		if (selected_phys & MGMT_PHY_EDR_2M_5SLOT)
  4110			pkt_type &= ~HCI_2DH5;
  4111		else
  4112			pkt_type |= HCI_2DH5;
  4113	
  4114		if (selected_phys & MGMT_PHY_EDR_3M_1SLOT)
  4115			pkt_type &= ~HCI_3DH1;
  4116		else
  4117			pkt_type |= HCI_3DH1;
  4118	
  4119		if (selected_phys & MGMT_PHY_EDR_3M_3SLOT)
  4120			pkt_type &= ~HCI_3DH3;
  4121		else
  4122			pkt_type |= HCI_3DH3;
  4123	
  4124		if (selected_phys & MGMT_PHY_EDR_3M_5SLOT)
  4125			pkt_type &= ~HCI_3DH5;
  4126		else
  4127			pkt_type |= HCI_3DH5;
  4128	
  4129		if (pkt_type != hdev->pkt_type) {
  4130			hdev->pkt_type = pkt_type;
  4131			changed = true;
  4132		}
  4133	
  4134		if ((selected_phys & MGMT_PHY_LE_MASK) ==
  4135		    (get_selected_phys(hdev) & MGMT_PHY_LE_MASK)) {
  4136			if (changed)
  4137				mgmt_phy_configuration_changed(hdev, sk);
  4138	
  4139			err = mgmt_cmd_complete(sk, hdev->id,
  4140						MGMT_OP_SET_PHY_CONFIGURATION,
  4141						0, NULL, 0);
  4142	
  4143			goto unlock;
  4144		}
  4145	
  4146		cmd = mgmt_pending_add(sk, MGMT_OP_SET_PHY_CONFIGURATION, hdev, data,
  4147				       len);
  4148		if (!cmd)
  4149			err = -ENOMEM;
  4150		else
  4151			err = hci_cmd_sync_queue(hdev, set_default_phy_sync, cmd,
  4152						 set_default_phy_complete);
  4153	
  4154		if (err < 0) {
  4155			err = mgmt_cmd_status(sk, hdev->id,
  4156					      MGMT_OP_SET_PHY_CONFIGURATION,
  4157					      MGMT_STATUS_FAILED);
  4158	
  4159			if (cmd)
  4160				mgmt_pending_remove(cmd);
  4161		}
  4162	
  4163	unlock:
  4164		hci_dev_unlock(hdev);
  4165	
  4166		return err;
  4167	}
  4168
diff mbox series

Patch

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4c421ebac669..9627528ed35f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4049,7 +4049,7 @@  static int set_phy_configuration(struct sock *sk, struct hci_dev *hdev,
 {
 	struct mgmt_cp_set_phy_configuration *cp = data;
 	struct mgmt_pending_cmd *cmd;
-	u32 selected_phys, configurable_phys, supported_phys, unconfigure_phys;
+	u32 selected_phys, configurable_phys, supported_phys;
 	u16 pkt_type = (HCI_DH1 | HCI_DM1);
 	bool changed = false;
 	int err;
@@ -4060,14 +4060,7 @@  static int set_phy_configuration(struct sock *sk, struct hci_dev *hdev,
 	supported_phys = get_supported_phys(hdev);
 	selected_phys = __le32_to_cpu(cp->selected_phys);
 
-	if (selected_phys & ~supported_phys)
-		return mgmt_cmd_status(sk, hdev->id,
-				       MGMT_OP_SET_PHY_CONFIGURATION,
-				       MGMT_STATUS_INVALID_PARAMS);
-
-	unconfigure_phys = supported_phys & ~configurable_phys;
-
-	if ((selected_phys & unconfigure_phys) != unconfigure_phys)
+	if (selected_phys & ~configurable_phys)
 		return mgmt_cmd_status(sk, hdev->id,
 				       MGMT_OP_SET_PHY_CONFIGURATION,
 				       MGMT_STATUS_INVALID_PARAMS);