All of lore.kernel.org
 help / color / mirror / Atom feed
From: zyw@rock-chips.com (Chris Zhong)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.
Date: Tue, 11 Nov 2014 15:10:57 +0800	[thread overview]
Message-ID: <5461B681.5020900@rock-chips.com> (raw)
In-Reply-To: <7hwq76pqyz.fsf@deeprootsystems.com>

Hi Kevin

On 11/08/2014 06:48 AM, Kevin Hilman wrote:
> Chris Zhong <zyw@rock-chips.com> writes:
>
>> RK3288 can shut down the cpu, gpu and other device controllers in suspend,
>> and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
>> process of suspend, pull the pin to low again when resume.
> The cover letter still doesn't state what this series applies to, or
> what its dependencies are for testing, even though it was requested in
> earlier reviews[1].  I discovered (again) by trial and error it applies
> to current linux-next.  I also discovered (as was earlier discussed[2])
> that it still does not resume using current upstream code, and those
> dependencies are not described here either.  These are the kinds of
> things that are crucial in a cover letter in order to help reviewers and
> testers not have to spend time digging through the archives trying to
> remember from the previous round of reviews.
Thank you for your review, I will perfect the cover letter in next 
version patches.

> Please, please list the out-of-tree dependencies, and how to test,
> including how you tested it, and on what hardware.
>
> Speaking of earlier reviews, I've noticed that after reviewing this
> series multiple times, you never respond to questions and/or comments.
> Two-way communication is important when collaborating on getting complex
> code this upstream, so please take some time to acknowledge the comments
> of reviewers and engage in discussion when questions are asked.  Even if
> it's as simple as "OK, I'll fix it in the next version", that helps
> reviewers know that they're not wasting their time.
OK, I'm going to respond all, even though only "Done".

> Kevin
>
> [1] https://lkml.org/lkml/2014/10/29/759
> [2] https://lkml.org/lkml/2014/10/29/881
>
>
>

WARNING: multiple messages have this Message-ID (diff)
From: Chris Zhong <zyw@rock-chips.com>
To: Kevin Hilman <khilman@kernel.org>
Cc: heiko@sntech.de, dianders@chromium.org, mturquette@linaro.org,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Russell King <linux@arm.linux.org.uk>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Kumar Gala <galak@codeaurora.org>,
	Tony Xie <xxx@rock-chips.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.
Date: Tue, 11 Nov 2014 15:10:57 +0800	[thread overview]
Message-ID: <5461B681.5020900@rock-chips.com> (raw)
In-Reply-To: <7hwq76pqyz.fsf@deeprootsystems.com>

Hi Kevin

On 11/08/2014 06:48 AM, Kevin Hilman wrote:
> Chris Zhong <zyw@rock-chips.com> writes:
>
>> RK3288 can shut down the cpu, gpu and other device controllers in suspend,
>> and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
>> process of suspend, pull the pin to low again when resume.
> The cover letter still doesn't state what this series applies to, or
> what its dependencies are for testing, even though it was requested in
> earlier reviews[1].  I discovered (again) by trial and error it applies
> to current linux-next.  I also discovered (as was earlier discussed[2])
> that it still does not resume using current upstream code, and those
> dependencies are not described here either.  These are the kinds of
> things that are crucial in a cover letter in order to help reviewers and
> testers not have to spend time digging through the archives trying to
> remember from the previous round of reviews.
Thank you for your review, I will perfect the cover letter in next 
version patches.

> Please, please list the out-of-tree dependencies, and how to test,
> including how you tested it, and on what hardware.
>
> Speaking of earlier reviews, I've noticed that after reviewing this
> series multiple times, you never respond to questions and/or comments.
> Two-way communication is important when collaborating on getting complex
> code this upstream, so please take some time to acknowledge the comments
> of reviewers and engage in discussion when questions are asked.  Even if
> it's as simple as "OK, I'll fix it in the next version", that helps
> reviewers know that they're not wasting their time.
OK, I'm going to respond all, even though only "Done".

> Kevin
>
> [1] https://lkml.org/lkml/2014/10/29/759
> [2] https://lkml.org/lkml/2014/10/29/881
>
>
>

  parent reply	other threads:[~2014-11-11  7:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-07 13:49 [PATCH v7 0/5] This is the 1st version of suspend for RK3288 Chris Zhong
2014-11-07 13:49 ` Chris Zhong
2014-11-07 13:49 ` Chris Zhong
2014-11-07 13:49 ` [PATCH v7 1/5] clk: rockchip: RK3288: add suspend and resume Chris Zhong
2014-11-07 13:49   ` Chris Zhong
2014-11-10 20:00   ` Heiko Stübner
2014-11-10 20:00     ` Heiko Stübner
2014-11-13  0:38   ` Mike Turquette
2014-11-13  0:38     ` Mike Turquette
2014-11-07 13:49 ` [PATCH v7 2/5] ARM: rockchip: add suspend and resume for RK3288 Chris Zhong
2014-11-07 13:49   ` Chris Zhong
2014-11-10 22:42   ` Doug Anderson
2014-11-10 22:42     ` Doug Anderson
2014-11-07 13:49 ` [PATCH v7 3/5] ARM: rockchip: Add pmu-sram binding Chris Zhong
2014-11-07 13:49 ` [PATCH v7 4/5] ARM: dts: add RK3288 suspend support Chris Zhong
2014-11-07 13:49   ` Chris Zhong
2014-11-10 18:40   ` Doug Anderson
2014-11-10 18:40     ` Doug Anderson
2014-11-10 18:40     ` Doug Anderson
2014-11-07 13:49 ` [PATCH v7 5/5] ARM: dts: add suspend voltage setting for RK808 Chris Zhong
2014-11-07 13:49   ` Chris Zhong
2014-11-07 22:48 ` [PATCH v7 0/5] This is the 1st version of suspend for RK3288 Kevin Hilman
2014-11-07 22:48   ` Kevin Hilman
2014-11-11  0:40   ` Heiko Stübner
2014-11-11  0:40     ` Heiko Stübner
2014-11-11  5:47     ` Doug Anderson
2014-11-11  5:47       ` Doug Anderson
2014-11-11  5:47       ` Doug Anderson
2014-11-11  7:10   ` Chris Zhong [this message]
2014-11-11  7:10     ` Chris Zhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5461B681.5020900@rock-chips.com \
    --to=zyw@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.