From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [net,2/2] net: usb: aqc111: Support for thermal throttling feature From: Andrew Lunn Message-Id: <20181212160811.GF1549@lunn.ch> Date: Wed, 12 Dec 2018 17:08:11 +0100 To: Igor Russkikh Cc: "linux-usb@vger.kernel.org" , "davem@davemloft.net" , "netdev@vger.kernel.org" , Dmitry Bezrukov List-ID: T24gV2VkLCBEZWMgMTIsIDIwMTggYXQgMDE6NTA6MTBQTSArMDAwMCwgSWdvciBSdXNza2lraCB3 cm90ZToKPiBGcm9tOiBEbWl0cnkgQmV6cnVrb3YgPGRtaXRyeS5iZXpydWtvdkBhcXVhbnRpYS5j b20+Cj4gCj4gTGFiIHRlc3Rpbmcgc2hvd3MgdGhhdCBjaGlwIG1heSBnZXQgb3ZlcmhlYXRlZCB3 aGVuCj4gbmV0d29yayBsaW5rIGlzIGNvbm5lY3RlZCBvbiBoaWdoIHNwZWVkICgyLjVHLzVHKS4K PiAKPiBEZWZhdWx0IGhhcmR3YXJlIGRlc2lnbiB1c2VzIG9ubHkgcGFzc2l2ZSBoZWF0c2luayB3 aXRob3V0IGEgY29vbGVyLAo+IGFuZCB0aGF0IG1ha2VzIHRoaW5ncyB3b3JzZS4KPiAKPiBUbyBw cmV2ZW50IHBvc3NpYmxlIGNoaXAgZGFtYWdlIGhlcmUgd2UgYWRkIHRocm90dGxpbmcgbWVjaGFu aXNtLgo+IAo+IFRoZXJlIGlzIGEgd29ya2VyIHRoYXQgbW9uaXRvcnMgdGhlIFBIWSdzIHRlbXBl cmF0dXJlIHZpYSByZWFkaW5nCj4gUEhZIHJlZ2lzdGVycy4gV2hlbiBQSFkncyB0ZW1wZXJhdHVy ZSByZWFjaGVzIHRoZSBjcml0aWNhbCB0aHJlc2hvbGQKPiAoaXQgaXMgMTA2IGRlZ3JlZXMgb2Yg Q2Vsc2l1cykgaXQgY2hhbmdlcyB0aGUgbGluayBzcGVlZCB0byB0aGUgbG93ZXN0Cj4gcmVnYXJk bGVzcyB1c2VyL2RlZmF1bHQgbGluayBzcGVlZCBzZXR0aW5ncy4gSXQgc2hvdWxkIHJlZHVjZSB0 aGUgUEhZJ3MKPiB0ZW1wZXJhdHVyZSB0byBub3JtYWwgbnVtYmVycy4KPiAKPiBXaGVuIHRoZSBQ SFkncyB0ZW1wYXJhdHVyZSBpcyBiYWNrIHRvIG5vcm1hbCAobG93IHRocmVzaG9sZCwgaXQgaXMK PiA4NSBkZWdyZWVzKSBpdCByZXN0b3JlcyB1c2VyL2RlZmF1bHQgbGluayBzcGVlZCBzZXR0aW5n cy4KCkhpIElnb3IKClBsZWFzZSBjb3VsZCB5b3UgYWxzbyBleHBvcnQgdGhlIHRlbXBlcmF0dXJl IHVzaW5nIEhXTU9OLiBUaGUgTWFydmVsbApQSFkgZHJpdmVycyBhcmUgZ29vZCBleGFtcGxlcy4K CkkgYWxzbyBoYXZlIGEgcGF0Y2ggZm9yIGRyaXZlci9uZXQvcGh5L2FxdWFudGlhLmMgd2hpY2gg YWRkcyBIV01PTgpzdXBwb3J0IHRvIHRoYXQgUEhZLgoKPiAKPiBTaWduZWQtb2ZmLWJ5OiBEbWl0 cnkgQmV6cnVrb3YgPGRtaXRyeS5iZXpydWtvdkBhcXVhbnRpYS5jb20+Cj4gU2lnbmVkLW9mZi1i eTogSWdvciBSdXNza2lraCA8aWdvci5ydXNza2lraEBhcXVhbnRpYS5jb20+Cj4gLS0tCj4gIGRy aXZlcnMvbmV0L3VzYi9hcWMxMTEuYyB8IDc4ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysKPiAgZHJpdmVycy9uZXQvdXNiL2FxYzExMS5oIHwgIDggKysrKysKPiAgMiBm aWxlcyBjaGFuZ2VkLCA4NiBpbnNlcnRpb25zKCspCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv bmV0L3VzYi9hcWMxMTEuYyBiL2RyaXZlcnMvbmV0L3VzYi9hcWMxMTEuYwo+IGluZGV4IDg4Njdm NmEzZWFhNy4uZmE2YjlkZmMyYTZmIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvbmV0L3VzYi9hcWMx MTEuYwo+ICsrKyBiL2RyaXZlcnMvbmV0L3VzYi9hcWMxMTEuYwo+IEBAIC0xNyw2ICsxNyw3IEBA Cj4gICNpbmNsdWRlIDxsaW51eC91c2IvY2RjLmg+Cj4gICNpbmNsdWRlIDxsaW51eC91c2IvdXNi bmV0Lmg+Cj4gICNpbmNsdWRlIDxsaW51eC9saW5rbW9kZS5oPgo+ICsjaW5jbHVkZSA8bGludXgv d29ya3F1ZXVlLmg+Cj4gIAo+ICAjaW5jbHVkZSAiYXFjMTExLmgiCj4gIAo+IEBAIC0zMzQsNiAr MzM1LDkgQEAgc3RhdGljIHZvaWQgYXFjMTExX3NldF9waHlfc3BlZWQoc3RydWN0IHVzYm5ldCAq ZGV2LCB1OCBhdXRvbmVnLCB1MTYgc3BlZWQpCj4gIAlhcWMxMTFfZGF0YS0+cGh5X2NmZyB8PSAo MyA8PCBBUV9EU0hfUkVUUklFU19TSElGVCkgJgo+ICAJCQkJQVFfRFNIX1JFVFJJRVNfTUFTSzsK PiAgCj4gKwlpZiAoYXFjMTExX2RhdGEtPnRoZXJtYWxfdGhyb3R0bGluZykKPiArCQlzcGVlZCA9 IFNQRUVEXzEwMDsKPiArCgpUaGF0IGlzIGEgYmlnIGp1bXAuIERvIHlvdSBuZWVkIHRvIGNvb2wg aXMgZG93biBxdWlja2x5LCBvciB3b3VsZAoxR2JwcyBiZSBzdWZmaWNpZW50PyBJIHRoaW5rIGFz IGEgdXNlciwgaSB3b3VsZCBwcmVmZXIgaXQgcGluZy1wb25ncwpiZXR3ZWVuIDVHIGFuZCAxRywg bm90IDVHIGFuZCAxMDBNLgoKPiAgCWlmIChhdXRvbmVnID09IEFVVE9ORUdfRU5BQkxFKSB7Cj4g IAkJc3dpdGNoIChzcGVlZCkgewo+ICAJCWNhc2UgU1BFRURfNTAwMDoKCkl0IGxvb2tzIGxpa2Ug dGhpcyBvbmx5IHdvcmtzIHdoZW4gYXV0by1uZWcgaXMgZW5hYmxlZC4gSWYgaSd2ZSBmaXhlZApj b25maWd1cmVkIGl0IGkgZG9uJ3QgdGhpbmsgdGhpcyB3b3Jrcz8KCj4gQEAgLTcxNCw2ICs3MTgs OCBAQCBzdGF0aWMgaW50IGFxYzExMV9iaW5kKHN0cnVjdCB1c2JuZXQgKmRldiwgc3RydWN0IHVz Yl9pbnRlcmZhY2UgKmludGYpCj4gIAkvKiBzdG9yZSBhcWMxMTFfZGF0YSBwb2ludGVyIGluIGRl dmljZSBkYXRhIGZpZWxkICovCj4gIAlkZXYtPmRyaXZlcl9wcml2ID0gYXFjMTExX2RhdGE7Cj4g IAo+ICsJYXFjMTExX2RhdGEtPmRldiA9IGRldjsKPiArCj4gIAkvKiBJbml0IHRoZSBNQUMgYWRk cmVzcyAqLwo+ICAJcmV0ID0gYXFjMTExX3JlYWRfcGVybV9tYWMoZGV2KTsKPiAgCWlmIChyZXQp Cj4gQEAgLTk5MSw2ICs5OTcsNzEgQEAgc3RhdGljIGludCBhcWMxMTFfbGlua19yZXNldChzdHJ1 Y3QgdXNibmV0ICpkZXYpCj4gIAlyZXR1cm4gMDsKPiAgfQo+ICAKPiAraW50IGFxYzExMV9nZXRf dGVtcGVyYXR1cmUoc3RydWN0IHVzYm5ldCAqZGV2LCB1MzIgKnRlbXBlcmF0dXJlKQo+ICt7Cj4g Kwl1MTYgcmVnMTYgPSAwOwo+ICsKPiArCWlmIChhcWMxMTFfbWRpb19yZWFkKGRldiwgQVFfR0xC X1RIRVJNQUxfU1RBVFVTMiwgQVFfUEhZX0dMT0JBTF9BRERSLAo+ICsJCQkgICAgICZyZWcxNikg PCAwKQo+ICsJCWdvdG8gZXJyOwo+ICsKPiArCWlmICghKHJlZzE2ICYgQVFfVEhFUk1fU1RfUkVB RFkpKQo+ICsJCWdvdG8gZXJyOwo+ICsKPiArCWlmIChhcWMxMTFfbWRpb19yZWFkKGRldiwgQVFf R0xCX1RIRVJNQUxfU1RBVFVTMSwgQVFfUEhZX0dMT0JBTF9BRERSLAo+ICsJCQkgICAgICZyZWcx NikgPCAwKQo+ICsJCWdvdG8gZXJyOwo+ICsKPiArCS8qY29udmVydCBmcm9tIDEvMjU2IHRvIDEv MTAwIGRlZ3JlZXMgb2YgQ2Vsc2l1cyovCj4gKwkqdGVtcGVyYXR1cmUgPSAodTMyKXJlZzE2ICog MTAwIC8gMjU2Owo+ICsJcmV0dXJuIDA7Cj4gKwo+ICtlcnI6Cj4gKwkqdGVtcGVyYXR1cmUgPSAw Owo+ICsJcmV0dXJuIC0xOwo+ICt9Cj4gKwo+ICt2b2lkIGFxYzExMV90aGVybWFsX3dvcmtfY2Io c3RydWN0IHdvcmtfc3RydWN0ICp3KQo+ICt7Cj4gKwlzdHJ1Y3QgZGVsYXllZF93b3JrICpkdyA9 IHRvX2RlbGF5ZWRfd29yayh3KTsKPiArCXN0cnVjdCBhcWMxMTFfZGF0YSAqYXFjMTExX2RhdGEg PSBjb250YWluZXJfb2YoZHcsIHN0cnVjdCBhcWMxMTFfZGF0YSwKPiArCQkJCQkJICAgICAgIHRo ZXJtX3dvcmspOwo+ICsJdW5zaWduZWQgbG9uZyB0aW1lb3V0ID0gbXNlY3NfdG9famlmZmllcyhB UV9USEVSTUFMX1RJTUVSX01TKTsKPiArCXN0cnVjdCB1c2JuZXQgKmRldiA9IGFxYzExMV9kYXRh LT5kZXY7Cj4gKwl1MzIgdGVtcGVyYXR1cmUgPSAwOwo+ICsJdTggcmVzZXRfc3BlZWQgPSAwOwo+ ICsKPiArCWlmICghYXFjMTExX2RhdGEtPmxpbmspCj4gKwkJLyogcG9sbCBub3Qgc28gZnJlcXVl bnRseSAqLwo+ICsJCXRpbWVvdXQgKj0gMjsKPiArCj4gKwlpZiAoYXFjMTExX2dldF90ZW1wZXJh dHVyZShkZXYsICZ0ZW1wZXJhdHVyZSkgIT0gMCkKPiArCQlnb3RvIGVuZDsKPiArCj4gKwlpZiAo YXFjMTExX2RhdGEtPnRoZXJtYWxfdGhyb3R0bGluZyAmJgo+ICsJICAgIHRlbXBlcmF0dXJlIDw9 IEFRX05PUk1BTF9URU1QX1RIUkVTSE9MRCkgewo+ICsJCW5ldGRldl9pbmZvKGRldi0+bmV0LCAi VGhlIHRlbXBlcmF0dXJlIGlzIGJhY2sgdG8gbm9ybWFsKCV1KSIsCj4gKwkJCSAgICBBUV9OT1JN QUxfVEVNUF9USFJFU0hPTEQgLyAxMDApOwoKSG93IG9mdGVuIGRvIHlvdSBzZWUgdGhlc2UgbWVz c2FnZXM/IEknbSB3b25kZXJpbmcgaWYgdGhleSBuZWVkIHRvIGJlCnJhdGUgbGltaXRlZD8KCj4g KwkJYXFjMTExX2RhdGEtPnRoZXJtYWxfdGhyb3R0bGluZyA9IDA7Cj4gKwkJcmVzZXRfc3BlZWQg PSAxOwo+ICsJfQo+ICsKPiArCWlmICghYXFjMTExX2RhdGEtPnRoZXJtYWxfdGhyb3R0bGluZyAm Jgo+ICsJICAgIHRlbXBlcmF0dXJlID49IEFRX0NSSVRJQ0FMX1RFTVBfVEhSRVNIT0xEKSB7CgpT aG91bGQgdGhlcmUgYmUgc29tZSBoeXN0ZXJlc2lzIGluIGhlcmU/IEluIGZhY3QsIGlmIHRlbXBl cmF0dXJlIGlzCkFRX0NSSVRJQ0FMX1RFTVBfVEhSRVNIT0xEIGl0IGlzIGJvdGggYmFjayB0byBu b3JtYWwsIGFuZCB0aHJvdHRsZWQgYXQKdGhlIHNhbWUgdGltZSEKCj4gKwkJbmV0ZGV2X3dhcm4o ZGV2LT5uZXQsICJDcml0aWNhbCB0ZW1wZXJhdHVyZSgldSkgaXMgcmVhY2hlZCIsCj4gKwkJCSAg ICBBUV9DUklUSUNBTF9URU1QX1RIUkVTSE9MRCAvIDEwMCk7Cj4gKwkJYXFjMTExX2RhdGEtPnRo ZXJtYWxfdGhyb3R0bGluZyA9IDE7Cj4gKwkJcmVzZXRfc3BlZWQgPSAxOwoKdXBkYXRlX3NwZWVk IG1pZ2h0IGJlIGEgYmV0dGVyIG5hbWUsIHNpbmNlIHlvdSBhcmUgbm90IGFsd2F5cwpyZXNldHRp bmcgaXQuCgoJICBBbmRyZXcK From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH net 2/2] net: usb: aqc111: Support for thermal throttling feature Date: Wed, 12 Dec 2018 17:08:11 +0100 Message-ID: <20181212160811.GF1549@lunn.ch> References: <658f5fab466face2aed2d53dabfe3af16595019a.1544622414.git.igor.russkikh@aquantia.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "linux-usb@vger.kernel.org" , "davem@davemloft.net" , "netdev@vger.kernel.org" , Dmitry Bezrukov To: Igor Russkikh Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:44652 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727845AbeLLQIQ (ORCPT ); Wed, 12 Dec 2018 11:08:16 -0500 Content-Disposition: inline In-Reply-To: <658f5fab466face2aed2d53dabfe3af16595019a.1544622414.git.igor.russkikh@aquantia.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Dec 12, 2018 at 01:50:10PM +0000, Igor Russkikh wrote: > From: Dmitry Bezrukov > > Lab testing shows that chip may get overheated when > network link is connected on high speed (2.5G/5G). > > Default hardware design uses only passive heatsink without a cooler, > and that makes things worse. > > To prevent possible chip damage here we add throttling mechanism. > > There is a worker that monitors the PHY's temperature via reading > PHY registers. When PHY's temperature reaches the critical threshold > (it is 106 degrees of Celsius) it changes the link speed to the lowest > regardless user/default link speed settings. It should reduce the PHY's > temperature to normal numbers. > > When the PHY's temparature is back to normal (low threshold, it is > 85 degrees) it restores user/default link speed settings. Hi Igor Please could you also export the temperature using HWMON. The Marvell PHY drivers are good examples. I also have a patch for driver/net/phy/aquantia.c which adds HWMON support to that PHY. > > Signed-off-by: Dmitry Bezrukov > Signed-off-by: Igor Russkikh > --- > drivers/net/usb/aqc111.c | 78 ++++++++++++++++++++++++++++++++++++++++ > drivers/net/usb/aqc111.h | 8 +++++ > 2 files changed, 86 insertions(+) > > diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c > index 8867f6a3eaa7..fa6b9dfc2a6f 100644 > --- a/drivers/net/usb/aqc111.c > +++ b/drivers/net/usb/aqc111.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include "aqc111.h" > > @@ -334,6 +335,9 @@ static void aqc111_set_phy_speed(struct usbnet *dev, u8 autoneg, u16 speed) > aqc111_data->phy_cfg |= (3 << AQ_DSH_RETRIES_SHIFT) & > AQ_DSH_RETRIES_MASK; > > + if (aqc111_data->thermal_throttling) > + speed = SPEED_100; > + That is a big jump. Do you need to cool is down quickly, or would 1Gbps be sufficient? I think as a user, i would prefer it ping-pongs between 5G and 1G, not 5G and 100M. > if (autoneg == AUTONEG_ENABLE) { > switch (speed) { > case SPEED_5000: It looks like this only works when auto-neg is enabled. If i've fixed configured it i don't think this works? > @@ -714,6 +718,8 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) > /* store aqc111_data pointer in device data field */ > dev->driver_priv = aqc111_data; > > + aqc111_data->dev = dev; > + > /* Init the MAC address */ > ret = aqc111_read_perm_mac(dev); > if (ret) > @@ -991,6 +997,71 @@ static int aqc111_link_reset(struct usbnet *dev) > return 0; > } > > +int aqc111_get_temperature(struct usbnet *dev, u32 *temperature) > +{ > + u16 reg16 = 0; > + > + if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS2, AQ_PHY_GLOBAL_ADDR, > + ®16) < 0) > + goto err; > + > + if (!(reg16 & AQ_THERM_ST_READY)) > + goto err; > + > + if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS1, AQ_PHY_GLOBAL_ADDR, > + ®16) < 0) > + goto err; > + > + /*convert from 1/256 to 1/100 degrees of Celsius*/ > + *temperature = (u32)reg16 * 100 / 256; > + return 0; > + > +err: > + *temperature = 0; > + return -1; > +} > + > +void aqc111_thermal_work_cb(struct work_struct *w) > +{ > + struct delayed_work *dw = to_delayed_work(w); > + struct aqc111_data *aqc111_data = container_of(dw, struct aqc111_data, > + therm_work); > + unsigned long timeout = msecs_to_jiffies(AQ_THERMAL_TIMER_MS); > + struct usbnet *dev = aqc111_data->dev; > + u32 temperature = 0; > + u8 reset_speed = 0; > + > + if (!aqc111_data->link) > + /* poll not so frequently */ > + timeout *= 2; > + > + if (aqc111_get_temperature(dev, &temperature) != 0) > + goto end; > + > + if (aqc111_data->thermal_throttling && > + temperature <= AQ_NORMAL_TEMP_THRESHOLD) { > + netdev_info(dev->net, "The temperature is back to normal(%u)", > + AQ_NORMAL_TEMP_THRESHOLD / 100); How often do you see these messages? I'm wondering if they need to be rate limited? > + aqc111_data->thermal_throttling = 0; > + reset_speed = 1; > + } > + > + if (!aqc111_data->thermal_throttling && > + temperature >= AQ_CRITICAL_TEMP_THRESHOLD) { Should there be some hysteresis in here? In fact, if temperature is AQ_CRITICAL_TEMP_THRESHOLD it is both back to normal, and throttled at the same time! > + netdev_warn(dev->net, "Critical temperature(%u) is reached", > + AQ_CRITICAL_TEMP_THRESHOLD / 100); > + aqc111_data->thermal_throttling = 1; > + reset_speed = 1; update_speed might be a better name, since you are not always resetting it. Andrew