| Message ID | 20220505015814.3727692-6-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PM: Solution for S0ix failure caused by PCH overheating | expand |
Zhang Rui <rui.zhang@intel.com> writes: > Remove the useless debug message for unsupported PM event because it is > noop in current code, and it gives a warning when a new event is > introduced, which it doesn't care. It's a debug message, not a warning, and only visible when debug messages are enabled. Why do you want to remove it? > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com> Is this really tested on a wil6210 device? Not that it matters, just surprised to see a Tested-by for a wil6210 patch. It's not really common hardware.
Hi, Kalle, thanks for the quick response. On Thu, 2022-05-05 at 07:38 +0300, Kalle Valo wrote: > Zhang Rui <rui.zhang@intel.com> writes: > > > Remove the useless debug message for unsupported PM event because > > it is > > noop in current code, and it gives a warning when a new event is > > introduced, which it doesn't care. > > It's a debug message, not a warning, and only visible when debug > messages are enabled. Why do you want to remove it? I'm concerning that people will report problems when they see new messages which never shows up previously. Deleting or keeping this message are both okay to me. But patch 6/7 indeed introduces a change to this piece of code and it's better for you to be aware of it before people starts to complain. > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com> > > Is this really tested on a wil6210 device? Not that it matters, just > surprised to see a Tested-by for a wil6210 patch. It's not really > common > hardware. No, we just tested the whole patch series on a Dell 9360 laptop, and a series of internal test machines. I didn't check if any of them has this device or not. Maybe I should remove the tested by in this case? thanks, rui
Zhang Rui <rui.zhang@intel.com> writes: > Hi, Kalle, > > thanks for the quick response. > > On Thu, 2022-05-05 at 07:38 +0300, Kalle Valo wrote: >> Zhang Rui <rui.zhang@intel.com> writes: >> >> > Remove the useless debug message for unsupported PM event because >> > it is >> > noop in current code, and it gives a warning when a new event is >> > introduced, which it doesn't care. >> >> It's a debug message, not a warning, and only visible when debug >> messages are enabled. Why do you want to remove it? > > I'm concerning that people will report problems when they see new > messages which never shows up previously. > > Deleting or keeping this message are both okay to me. But patch 6/7 > indeed introduces a change to this piece of code and it's better for > you to be aware of it before people starts to complain. > >> >> > Signed-off-by: Zhang Rui <rui.zhang@intel.com> >> > Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com> >> >> Is this really tested on a wil6210 device? Not that it matters, just >> surprised to see a Tested-by for a wil6210 patch. It's not really >> common >> hardware. > > No, we just tested the whole patch series on a Dell 9360 laptop, and a > series of internal test machines. I didn't check if any of them has > this device or not. Maybe I should remove the tested by in this case? I think it's best to drop this wil6210 patch. The driver is orphaned anyway and if anyone complains, they will do that to me :)
On Fri, 2022-05-06 at 17:04 +0300, Kalle Valo wrote: > Zhang Rui <rui.zhang@intel.com> writes: > > > Hi, Kalle, > > > > thanks for the quick response. > > > > On Thu, 2022-05-05 at 07:38 +0300, Kalle Valo wrote: > > > Zhang Rui <rui.zhang@intel.com> writes: > > > > > > > Remove the useless debug message for unsupported PM event > > > > because > > > > it is > > > > noop in current code, and it gives a warning when a new event > > > > is > > > > introduced, which it doesn't care. > > > > > > It's a debug message, not a warning, and only visible when debug > > > messages are enabled. Why do you want to remove it? > > > > I'm concerning that people will report problems when they see new > > messages which never shows up previously. > > > > Deleting or keeping this message are both okay to me. But patch 6/7 > > indeed introduces a change to this piece of code and it's better > > for > > you to be aware of it before people starts to complain. > > > > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > Tested-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com> > > > > > > Is this really tested on a wil6210 device? Not that it matters, > > > just > > > surprised to see a Tested-by for a wil6210 patch. It's not really > > > common > > > hardware. > > > > No, we just tested the whole patch series on a Dell 9360 laptop, > > and a > > series of internal test machines. I didn't check if any of them has > > this device or not. Maybe I should remove the tested by in this > > case? > > I think it's best to drop this wil6210 patch. The driver is orphaned > anyway and if anyone complains, they will do that to me :) > Sure, I will drop this patch. Thanks for reviewing. -rui
diff --git a/drivers/net/wireless/ath/wil6210/pcie_bus.c b/drivers/net/wireless/ath/wil6210/pcie_bus.c index ce40d94909ad..1d6c4e926004 100644 --- a/drivers/net/wireless/ath/wil6210/pcie_bus.c +++ b/drivers/net/wireless/ath/wil6210/pcie_bus.c @@ -600,7 +600,6 @@ static int wil6210_pm_notify(struct notifier_block *notify_block, evt); break; default: - wil_dbg_pm(wil, "unhandled notify mode %ld\n", mode); break; }