Message ID | 1321364453-15835-1-git-send-email-bertrand.cachet@heig-vd.ch |
---|---|
State | Accepted |
Commit | 3c8849df5f2954944578cd2e2af31c001283dbf2 |
Delegated to: | Stefano Babic |
Headers | show |
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
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);
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(-)