[U-Boot] [PATCH v4] Improve Power Management in SMC911X driver.
From datasheet, when READY bit is set inside PM_CTRL register, it means that
device is already in *normal* (D0) mode => it doesn't need to be wake-up.
With this patch, we only wake-up (writing on TEST_BYTE register) if PM_MODE bits of PM_CTRL register is in sleep (D1/D2) mode.
Signed-off-by: Bertrand Cachet bertrand.cachet@heig-vd.ch --- v2: Improve code styling by grouping two narrow comments. v3: Place versions information under scissors line. v4: Use tabs instead of spaces (vimrc problem)
drivers/net/smc911x.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 8ce08a9..a290073 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -471,8 +471,11 @@ static void smc911x_reset(struct eth_device *dev) { int timeout;
- /* Take out of PM setting first */ - if (smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) { + /* + * Take out of PM setting first + * Device is already wake up if PMT_CTRL_READY bit is set + */ + if ((smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) == 0) { /* Write to the bytetest will take out of powerdown */ smc911x_reg_write(dev, BYTE_TEST, 0x0);
On 11/15/2011 02:40 PM, Bertrand Cachet wrote:
From datasheet, when READY bit is set inside PM_CTRL register, it means that device is already in *normal* (D0) mode => it doesn't need to be wake-up.
With this patch, we only wake-up (writing on TEST_BYTE register) if PM_MODE bits of PM_CTRL register is in sleep (D1/D2) mode.
Signed-off-by: Bertrand Cachet bertrand.cachet@heig-vd.ch
v2: Improve code styling by grouping two narrow comments. v3: Place versions information under scissors line. v4: Use tabs instead of spaces (vimrc problem)
drivers/net/smc911x.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 8ce08a9..a290073 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -471,8 +471,11 @@ static void smc911x_reset(struct eth_device *dev) { int timeout;
- /* Take out of PM setting first */
- if (smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) {
- /*
* Take out of PM setting first* Device is already wake up if PMT_CTRL_READY bit is set*/- if ((smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) == 0) { /* Write to the bytetest will take out of powerdown */ smc911x_reg_write(dev, BYTE_TEST, 0x0);
Agree, this is the correct behavior. However, checkpatch report errors (due to leading white space instead of tabs in the comment you added):
ERROR: code indent should use tabs where possible #29: FILE: drivers/net/smc911x.h:475: + ^I * Take out of PM setting first$
WARNING: please, no space before tabs #29: FILE: drivers/net/smc911x.h:475: + ^I * Take out of PM setting first$
ERROR: code indent should use tabs where possible #30: FILE: drivers/net/smc911x.h:476: + ^I * Device is already wake up if PMT_CTRL_READY bit is set$
WARNING: please, no space before tabs #30: FILE: drivers/net/smc911x.h:476: + ^I * Device is already wake up if PMT_CTRL_READY bit is set$
total: 2 errors, 2 warnings, 13 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
Can you fix and repost (maybe setting me as CC, I will take care of your patch).
Thanks, Stefano Babic
Hi Stefano, Bertrand,
On 11/17/11 17:52, Stefano Babic wrote:
On 11/15/2011 02:40 PM, Bertrand Cachet wrote:
From datasheet, when READY bit is set inside PM_CTRL register, it means that device is already in *normal* (D0) mode => it doesn't need to be wake-up.
With this patch, we only wake-up (writing on TEST_BYTE register) if PM_MODE bits of PM_CTRL register is in sleep (D1/D2) mode.
Signed-off-by: Bertrand Cachet bertrand.cachet@heig-vd.ch
v2: Improve code styling by grouping two narrow comments. v3: Place versions information under scissors line. v4: Use tabs instead of spaces (vimrc problem)
drivers/net/smc911x.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 8ce08a9..a290073 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -471,8 +471,11 @@ static void smc911x_reset(struct eth_device *dev) { int timeout;
- /* Take out of PM setting first */
- if (smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) {
- /*
* Take out of PM setting first* Device is already wake up if PMT_CTRL_READY bit is set*/- if ((smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) == 0) { /* Write to the bytetest will take out of powerdown */ smc911x_reg_write(dev, BYTE_TEST, 0x0);
Agree, this is the correct behavior. However, checkpatch report errors (due to leading white space instead of tabs in the comment you added):
Are you sure you are testing the right version (v4)? Because, I get different errors/warnings on that patch (see below)
ERROR: code indent should use tabs where possible #29: FILE: drivers/net/smc911x.h:475:
- ^I * Take out of PM setting first$
WARNING: please, no space before tabs #29: FILE: drivers/net/smc911x.h:475:
- ^I * Take out of PM setting first$
ERROR: code indent should use tabs where possible #30: FILE: drivers/net/smc911x.h:476:
- ^I * Device is already wake up if PMT_CTRL_READY bit is set$
WARNING: please, no space before tabs #30: FILE: drivers/net/smc911x.h:476:
- ^I * Device is already wake up if PMT_CTRL_READY bit is set$
total: 2 errors, 2 warnings, 13 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
My checkpatch.pl reports: ----------------cut---------------- ERROR: trailing whitespace #133: FILE: drivers/net/smc911x.h:474: +^I/*^M$
ERROR: trailing whitespace #134: FILE: drivers/net/smc911x.h:475: +^I * Take out of PM setting first^M$
ERROR: trailing whitespace #135: FILE: drivers/net/smc911x.h:476: +^I * Device is already wake up if PMT_CTRL_READY bit is set^M$
ERROR: DOS line endings #136: FILE: drivers/net/smc911x.h:477: +^I */^M$
ERROR: DOS line endings #137: FILE: drivers/net/smc911x.h:478: +^Iif ((smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) == 0) {^M$
total: 5 errors, 0 warnings, 13 lines checked ---------------cut-----------------------------
Anyway, those need to be fixed.
On 11/17/2011 05:21 PM, Igor Grinberg wrote:
Agree, this is the correct behavior. However, checkpatch report errors (due to leading white space instead of tabs in the comment you added):
Are you sure you are testing the right version (v4)? Because, I get different errors/warnings on that patch (see below)
The right version, yes. Maybe I have a an old checkpatch.pl, I must update it.
Anyway, those need to be fixed.
Right.
On 15/11/2011 14:40, Bertrand Cachet wrote:
From datasheet, when READY bit is set inside PM_CTRL register, it means that device is already in *normal* (D0) mode => it doesn't need to be wake-up.
With this patch, we only wake-up (writing on TEST_BYTE register) if PM_MODE bits of PM_CTRL register is in sleep (D1/D2) mode.
Signed-off-by: Bertrand Cachet bertrand.cachet@heig-vd.ch
Applied to u-boot-staging, sbabic@denx.de, thanks.
Best regards, Stefano Babic
participants (3)
-
Bertrand Cachet -
Igor Grinberg -
Stefano Babic