From: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Barber <smbarber-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Subject: Re: [PATCH 1/3] thermal: handle get_temp() errors properly
Date: Tue, 22 Nov 2016 14:27:13 -0800 [thread overview]
Message-ID: <20161122222712.GA53509@google.com> (raw)
In-Reply-To: <20161122110045.GB2018-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Tue, Nov 22, 2016 at 03:00:47AM -0800, Eduardo Valentin wrote:
> On Tue, Nov 22, 2016 at 03:52:25PM +0800, Zhang Rui wrote:
> > On Fri, 2016-11-18 at 21:30 -0800, Brian Norris wrote:
> > > On Fri, Nov 18, 2016 at 07:41:59PM -0800, Eduardo Valentin wrote:
> > > > I would prefer we consider the patch I sent
> > > > some time ago:
> > > > https://patchwork.kernel.org/patch/7876381/
> > > Honestly I didn't look that deeply into the framework here (and I
> > > also
> > > don't use CONFIG_THERMAL_EMULATION), I was just fixing something that
> > > was obviously wrong.
>
> Yeah, but that is why we need people to look the code considering all
> features. :-)
Well, there are bugfixes and there are features. My patch fixed the bug
in the simplest way possible; it didn't break CONFIG_THERMAL_EMULATION
any further than it already was, and it'll still work if get_temp()
doesn't return an error.
I'd say your patch is essentially adding a feature, and IMO that's not
the best way to fix a bug. You can fix the bug and *then* add the
feature.
Anyway, I'm not going to tell you how to run your subsystem. If your
patch goes through, that's probably just as well.
[...]
> > hmmm, I forgot why I missed this one in the end.
> > Eduardo,
> > would you mind refresh and resend the patch?
>
> Yeah sure. I have at least three extra patch sets on thermal core on
> my queue. But I would like to get first the thermal sysfs reorg in
> first. This fix is one of the changes that will go on top of the thermal
> sysfs reorg.
So, the bugfix depends on feature work? I guess I'll check back in
another year to see what the status of the bugfix is :)
Brian
WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris@chromium.org>
To: Eduardo Valentin <edubezval@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>, Heiko Stuebner <heiko@sntech.de>,
linux-pm@vger.kernel.org, linux-rockchip@lists.infradead.org,
linux-kernel@vger.kernel.org, Caesar Wang <wxt@rock-chips.com>,
Stephen Barber <smbarber@chromium.org>
Subject: Re: [PATCH 1/3] thermal: handle get_temp() errors properly
Date: Tue, 22 Nov 2016 14:27:13 -0800 [thread overview]
Message-ID: <20161122222712.GA53509@google.com> (raw)
In-Reply-To: <20161122110045.GB2018@localhost.localdomain>
On Tue, Nov 22, 2016 at 03:00:47AM -0800, Eduardo Valentin wrote:
> On Tue, Nov 22, 2016 at 03:52:25PM +0800, Zhang Rui wrote:
> > On Fri, 2016-11-18 at 21:30 -0800, Brian Norris wrote:
> > > On Fri, Nov 18, 2016 at 07:41:59PM -0800, Eduardo Valentin wrote:
> > > > I would prefer we consider the patch I sent
> > > > some time ago:
> > > > https://patchwork.kernel.org/patch/7876381/
> > > Honestly I didn't look that deeply into the framework here (and I
> > > also
> > > don't use CONFIG_THERMAL_EMULATION), I was just fixing something that
> > > was obviously wrong.
>
> Yeah, but that is why we need people to look the code considering all
> features. :-)
Well, there are bugfixes and there are features. My patch fixed the bug
in the simplest way possible; it didn't break CONFIG_THERMAL_EMULATION
any further than it already was, and it'll still work if get_temp()
doesn't return an error.
I'd say your patch is essentially adding a feature, and IMO that's not
the best way to fix a bug. You can fix the bug and *then* add the
feature.
Anyway, I'm not going to tell you how to run your subsystem. If your
patch goes through, that's probably just as well.
[...]
> > hmmm, I forgot why I missed this one in the end.
> > Eduardo,
> > would you mind refresh and resend the patch?
>
> Yeah sure. I have at least three extra patch sets on thermal core on
> my queue. But I would like to get first the thermal sysfs reorg in
> first. This fix is one of the changes that will go on top of the thermal
> sysfs reorg.
So, the bugfix depends on feature work? I guess I'll check back in
another year to see what the status of the bugfix is :)
Brian
next prev parent reply other threads:[~2016-11-22 22:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-18 23:52 [PATCH 1/3] thermal: handle get_temp() errors properly Brian Norris
2016-11-18 23:52 ` [PATCH 2/3] thermal: rockchip: improve conversion error messages Brian Norris
2016-11-19 3:31 ` Caesar Wang
2016-11-19 3:35 ` Caesar Wang
2016-11-22 1:51 ` Caesar Wang
2016-11-22 2:15 ` Brian Norris
2016-11-22 2:33 ` Caesar Wang
2016-11-22 3:43 ` Brian Norris
2016-11-22 7:57 ` Zhang Rui
2016-11-22 12:44 ` Caesar Wang
[not found] ` <1479513177-81504-1-git-send-email-briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2016-11-18 23:52 ` [PATCH 3/3] thermal: rockchip: don't pass table structs by value Brian Norris
2016-11-18 23:52 ` Brian Norris
2016-11-19 4:03 ` Caesar Wang
2016-11-19 3:21 ` [PATCH 1/3] thermal: handle get_temp() errors properly Caesar Wang
2016-11-19 3:41 ` Eduardo Valentin
2016-11-19 5:30 ` Brian Norris
2016-11-22 7:52 ` Zhang Rui
2016-11-22 11:00 ` Eduardo Valentin
[not found] ` <20161122110045.GB2018-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2016-11-22 22:27 ` Brian Norris [this message]
2016-11-22 22:27 ` Brian Norris
2017-09-08 18:15 ` Dmitry Torokhov
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=20161122222712.GA53509@google.com \
--to=briannorris-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
--cc=edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=smbarber-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
--cc=wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.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.