diff mbox series

[2/4] mt76: remove wait argument from mt76x02_mcu_calibrate

Message ID 1540555229-28582-3-git-send-email-sgruszka@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series mt76: mcu api clenups | expand

Commit Message

Stanislaw Gruszka Oct. 26, 2018, noon UTC 
We always wait for CMD_CALIBRATION_OP mcu message, but wait argument is used
for do additioanl MT_MCU_COM_REG0 register operations, which are needed
for MMIO devices and we can use mt76_is_mmio() check instead of wait argument.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76x0/phy.c    | 14 ++++++--------
 drivers/net/wireless/mediatek/mt76/mt76x02_mcu.c   |  7 +++----
 drivers/net/wireless/mediatek/mt76/mt76x02_mcu.h   |  3 +--
 drivers/net/wireless/mediatek/mt76/mt76x2/mt76x2.h |  2 +-
 .../net/wireless/mediatek/mt76/mt76x2/pci_phy.c    | 22 +++++++++++-----------
 drivers/net/wireless/mediatek/mt76/mt76x2/phy.c    |  4 ++--
 .../net/wireless/mediatek/mt76/mt76x2/usb_phy.c    | 20 ++++++++++----------
 7 files changed, 34 insertions(+), 38 deletions(-)

Comments

Felix Fietkau Nov. 1, 2018, 5:29 p.m. UTC | #1 
On 2018-10-26 14:00, Stanislaw Gruszka wrote:
> We always wait for CMD_CALIBRATION_OP mcu message, but wait argument is used
> for do additioanl MT_MCU_COM_REG0 register operations, which are needed
> for MMIO devices and we can use mt76_is_mmio() check instead of wait argument.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
With this commit, I hit the WARN_ON on polling the MT_MCU_COM_REG0
register on mt76x0e. It seems to me that this register polling should be
moved out of mt76x02 and into a mt76x2 wrapper function.

- Felix
Stanislaw Gruszka Nov. 2, 2018, 12:56 p.m. UTC | #2 
On Thu, Nov 01, 2018 at 06:29:01PM +0100, Felix Fietkau wrote:
> On 2018-10-26 14:00, Stanislaw Gruszka wrote:
> > We always wait for CMD_CALIBRATION_OP mcu message, but wait argument is used
> > for do additioanl MT_MCU_COM_REG0 register operations, which are needed
> > for MMIO devices and we can use mt76_is_mmio() check instead of wait argument.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> With this commit, I hit the WARN_ON on polling the MT_MCU_COM_REG0
> register on mt76x0e. It seems to me that this register polling should be
> moved out of mt76x02 and into a mt76x2 wrapper function.

I think it would be better to just do mt76x2e check like this:

        bool is_mt76x2e = mt76_is_mmio(dev) && is_mt76x2(dev);

        if (is_mt76x2e)
                mt76_rmw(dev, MT_MCU_COM_REG0, BIT(31), 0);

Then we will not have sparate calibration functions for mt76x2e and
mt76x2u, so code could be easier shared between those.

Thanks
Stanislaw
Felix Fietkau Nov. 2, 2018, 4:07 p.m. UTC | #3 
On 2018-11-02 13:56, Stanislaw Gruszka wrote:
> On Thu, Nov 01, 2018 at 06:29:01PM +0100, Felix Fietkau wrote:
>> On 2018-10-26 14:00, Stanislaw Gruszka wrote:
>> > We always wait for CMD_CALIBRATION_OP mcu message, but wait argument is used
>> > for do additioanl MT_MCU_COM_REG0 register operations, which are needed
>> > for MMIO devices and we can use mt76_is_mmio() check instead of wait argument.
>> > 
>> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>> With this commit, I hit the WARN_ON on polling the MT_MCU_COM_REG0
>> register on mt76x0e. It seems to me that this register polling should be
>> moved out of mt76x02 and into a mt76x2 wrapper function.
> 
> I think it would be better to just do mt76x2e check like this:
> 
>         bool is_mt76x2e = mt76_is_mmio(dev) && is_mt76x2(dev);
> 
>         if (is_mt76x2e)
>                 mt76_rmw(dev, MT_MCU_COM_REG0, BIT(31), 0);
> 
> Then we will not have sparate calibration functions for mt76x2e and
> mt76x2u, so code could be easier shared between those.
My suggestion would have been making a wrapper function that checks for
mt76_is_mmio(dev) and gets put into mt76x2-common. What you're proposing
is fine with me as well, I guess it's a matter of preference.

- Felix
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
index ca24b5716b58..42351179c3cc 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
@@ -529,9 +529,8 @@  void mt76x0_phy_calibrate(struct mt76x02_dev *dev, bool power_on)
 		return;
 
 	if (power_on) {
-		mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0, false);
-		mt76x02_mcu_calibrate(dev, MCU_CAL_VCO, chan->hw_value,
-				      false);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_VCO, chan->hw_value);
 		usleep_range(10, 20);
 		/* XXX: tssi */
 	}
@@ -554,14 +553,14 @@  void mt76x0_phy_calibrate(struct mt76x02_dev *dev, bool power_on)
 		val = 0x600;
 	}
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_FULL, val, false);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_FULL, val);
 	msleep(350);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_LC, is_5ghz, false);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_LC, is_5ghz);
 	usleep_range(15000, 20000);
 
 	mt76_wr(dev, MT_BBP(IBI, 9), reg_val);
 	mt76_wr(dev, MT_TX_ALC_CFG_0, tx_alc);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, 1, false);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, 1);
 }
 EXPORT_SYMBOL_GPL(mt76x0_phy_calibrate);
 
@@ -698,8 +697,7 @@  static void mt76x0_phy_temp_sensor(struct mt76x02_dev *dev)
 
 	if (abs(val - dev->cal.temp_vco) > 20) {
 		mt76x02_mcu_calibrate(dev, MCU_CAL_VCO,
-				      dev->mt76.chandef.chan->hw_value,
-				      false);
+				      dev->mt76.chandef.chan->hw_value);
 		dev->cal.temp_vco = val;
 	}
 	if (abs(val - dev->cal.temp) > 30) {
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.c
index 550e73f81438..254bf3d22d94 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.c
@@ -165,8 +165,7 @@  int mt76x02_mcu_set_radio_state(struct mt76x02_dev *dev, bool on,
 }
 EXPORT_SYMBOL_GPL(mt76x02_mcu_set_radio_state);
 
-int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type,
-			  u32 param, bool wait)
+int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type, u32 param)
 {
 	struct {
 		__le32 id;
@@ -177,7 +176,7 @@  int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type,
 	};
 	int ret;
 
-	if (wait)
+	if (mt76_is_mmio(dev))
 		mt76_rmw(dev, MT_MCU_COM_REG0, BIT(31), 0);
 
 	ret = mt76_mcu_send_msg(dev, CMD_CALIBRATION_OP, &msg, sizeof(msg),
@@ -185,7 +184,7 @@  int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type,
 	if (ret)
 		return ret;
 
-	if (wait &&
+	if (mt76_is_mmio(dev) &&
 	    WARN_ON(!mt76_poll_msec(dev, MT_MCU_COM_REG0,
 				    BIT(31), BIT(31), 100)))
 		return -ETIMEDOUT;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.h
index a3aee579d42c..605cdb44961a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mcu.h
@@ -97,8 +97,7 @@  struct mt76x02_patch_header {
 };
 
 int mt76x02_mcu_cleanup(struct mt76x02_dev *dev);
-int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type,
-			  u32 param, bool wait);
+int mt76x02_mcu_calibrate(struct mt76x02_dev *dev, int type, u32 param);
 int mt76x02_mcu_msg_send(struct mt76_dev *mdev, int cmd, const void *data,
 			 int len, bool wait_resp);
 int mt76x02_mcu_function_select(struct mt76x02_dev *dev,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/mt76x2.h b/drivers/net/wireless/mediatek/mt76/mt76x2/mt76x2.h
index e5846a85d6ab..b259e4b50f1e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/mt76x2.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/mt76x2.h
@@ -78,7 +78,7 @@  void mt76x2_init_txpower(struct mt76x02_dev *dev,
 			 struct ieee80211_supported_band *sband);
 void mt76_write_mac_initvals(struct mt76x02_dev *dev);
 
-void mt76x2_phy_tssi_compensate(struct mt76x02_dev *dev, bool wait);
+void mt76x2_phy_tssi_compensate(struct mt76x02_dev *dev);
 void mt76x2_phy_set_txpower_regs(struct mt76x02_dev *dev,
 				 enum nl80211_band band);
 void mt76x2_configure_tx_delay(struct mt76x02_dev *dev,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
index 16ff6c376373..59ef7931df50 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
@@ -38,7 +38,7 @@  mt76x2_phy_tssi_init_cal(struct mt76x02_dev *dev)
 	if (mt76x02_ext_pa_enabled(dev, chan->band))
 		flag |= BIT(8);
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TSSI, flag, true);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TSSI, flag);
 	dev->cal.tssi_cal_done = true;
 	return true;
 }
@@ -62,13 +62,13 @@  mt76x2_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 		mt76x2_mac_stop(dev, false);
 
 	if (is_5ghz)
-		mt76x02_mcu_calibrate(dev, MCU_CAL_LC, 0, true);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_LC, 0);
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_LOFT, is_5ghz, true);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TXIQ, is_5ghz, true);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_RXIQC_FI, is_5ghz, true);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TEMP_SENSOR, 0, true);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_SHAPING, 0, true);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_LOFT, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TXIQ, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_RXIQC_FI, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TEMP_SENSOR, 0);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_SHAPING, 0);
 
 	if (!mac_stopped)
 		mt76x2_mac_resume(dev);
@@ -223,14 +223,14 @@  int mt76x2_phy_set_channel(struct mt76x02_dev *dev,
 		u8 val = mt76x02_eeprom_get(dev, MT_EE_BT_RCAL_RESULT);
 
 		if (val != 0xff)
-			mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0, true);
+			mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0);
 	}
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, channel, true);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, channel);
 
 	/* Rx LPF calibration */
 	if (!dev->cal.init_cal_done)
-		mt76x02_mcu_calibrate(dev, MCU_CAL_RC, 0, true);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_RC, 0);
 
 	dev->cal.init_cal_done = true;
 
@@ -294,7 +294,7 @@  void mt76x2_phy_calibrate(struct work_struct *work)
 
 	dev = container_of(work, struct mt76x02_dev, cal_work.work);
 	mt76x2_phy_channel_calibrate(dev, false);
-	mt76x2_phy_tssi_compensate(dev, true);
+	mt76x2_phy_tssi_compensate(dev);
 	mt76x2_phy_temp_compensate(dev);
 	mt76x2_phy_update_channel_gain(dev);
 	ieee80211_queue_delayed_work(mt76_hw(dev), &dev->cal_work,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
index bbeff9c19997..3ac5dd0f0400 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
@@ -210,7 +210,7 @@  void mt76x2_configure_tx_delay(struct mt76x02_dev *dev,
 }
 EXPORT_SYMBOL_GPL(mt76x2_configure_tx_delay);
 
-void mt76x2_phy_tssi_compensate(struct mt76x02_dev *dev, bool wait)
+void mt76x2_phy_tssi_compensate(struct mt76x02_dev *dev)
 {
 	struct ieee80211_channel *chan = dev->mt76.chandef.chan;
 	struct mt76x2_tx_power_info txp;
@@ -245,7 +245,7 @@  void mt76x2_phy_tssi_compensate(struct mt76x02_dev *dev, bool wait)
 			return;
 
 		usleep_range(10000, 20000);
-		mt76x02_mcu_calibrate(dev, MCU_CAL_DPD, chan->hw_value, wait);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_DPD, chan->hw_value);
 		dev->cal.dpd_cal_done = true;
 	}
 }
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
index 8011c261c658..2a2de7bee6b2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
@@ -29,12 +29,12 @@  void mt76x2u_phy_channel_calibrate(struct mt76x02_dev *dev)
 	mt76x2u_mac_stop(dev);
 
 	if (is_5ghz)
-		mt76x02_mcu_calibrate(dev, MCU_CAL_LC, 0, false);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_LC, 0);
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_LOFT, is_5ghz, false);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TXIQ, is_5ghz, false);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_RXIQC_FI, is_5ghz, false);
-	mt76x02_mcu_calibrate(dev, MCU_CAL_TEMP_SENSOR, 0, false);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TX_LOFT, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TXIQ, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_RXIQC_FI, is_5ghz);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_TEMP_SENSOR, 0);
 
 	mt76x2u_mac_resume(dev);
 }
@@ -44,7 +44,7 @@  void mt76x2u_phy_calibrate(struct work_struct *work)
 	struct mt76x02_dev *dev;
 
 	dev = container_of(work, struct mt76x02_dev, cal_work.work);
-	mt76x2_phy_tssi_compensate(dev, false);
+	mt76x2_phy_tssi_compensate(dev);
 	mt76x2_phy_update_channel_gain(dev);
 
 	ieee80211_queue_delayed_work(mt76_hw(dev), &dev->cal_work,
@@ -142,14 +142,14 @@  int mt76x2u_phy_set_channel(struct mt76x02_dev *dev,
 		u8 val = mt76x02_eeprom_get(dev, MT_EE_BT_RCAL_RESULT);
 
 		if (val != 0xff)
-			mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0, false);
+			mt76x02_mcu_calibrate(dev, MCU_CAL_R, 0);
 	}
 
-	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, channel, false);
+	mt76x02_mcu_calibrate(dev, MCU_CAL_RXDCOC, channel);
 
 	/* Rx LPF calibration */
 	if (!dev->cal.init_cal_done)
-		mt76x02_mcu_calibrate(dev, MCU_CAL_RC, 0, false);
+		mt76x02_mcu_calibrate(dev, MCU_CAL_RC, 0);
 	dev->cal.init_cal_done = true;
 
 	mt76_wr(dev, MT_BBP(AGC, 61), 0xff64a4e2);
@@ -182,7 +182,7 @@  int mt76x2u_phy_set_channel(struct mt76x02_dev *dev,
 				flag |= BIT(0);
 			if (mt76x02_ext_pa_enabled(dev, chan->band))
 				flag |= BIT(8);
-			mt76x02_mcu_calibrate(dev, MCU_CAL_TSSI, flag, false);
+			mt76x02_mcu_calibrate(dev, MCU_CAL_TSSI, flag);
 			dev->cal.tssi_cal_done = true;
 		}
 	}