linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Ladislav Michl <ladis@linux-mips.org>
Cc: linux-iio@vger.kernel.org,
	Daniel Baluta <daniel.baluta@intel.com>,
	Matt Ranostay <matt.ranostay@intel.com>
Subject: Re: [PATCH 2/2] iio: adc: ti-ads1015: use dynamically generated attributes
Date: Sat, 12 Aug 2017 12:10:44 +0100	[thread overview]
Message-ID: <20170812121044.034e2c38@archlinux> (raw)
In-Reply-To: <20170809164426.4zpihixosqlg2jtc@lenoch>

On Wed, 9 Aug 2017 18:44:26 +0200
Ladislav Michl <ladis@linux-mips.org> wrote:

> Hello Jonathan,
> 
> On Wed, Aug 09, 2017 at 03:11:00PM +0100, Jonathan Cameron wrote:
> > On Tue, 1 Aug 2017 10:26:46 +0200
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> >   
> > > Generate attributes dynamically based on actual values used
> > > by driver - leads to slightly smaller code (on ARM and X64)
> > > Scale and sampling frequency definitions are at one place now.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>  
> > Sorry, for the small gain I am unconvinced.  If this was a new
> > driver and it had been submitted this way I wouldn't be asking
> > for it to be changed, but I'm also not interested in the churn and
> > additional code for what must be a pretty small chance.  
> 
> As stated in cover letter I was not really considering it to be
> a huge improvement and in fact did it as excercise to better
> uderstand iio, so I'm not afraid it doesn't go in.
Ooops. The fact I don't always read cover letters becomes apparent ;)

Reviewers are lazy, put comments about a particular patch in that
patch then there is a slightly larger chance that people will notice
it.
> 
> > Also if you really want to do tinification patches, please post
> > the actual sizes of the various regions.  In particular you need
> > to generally have a fairly large saving to justify replacing
> > stuff that is constant with code.  
> 
> I'll see if something better can be done based on fixes-togreg branch.
Cool.
> 
> Thank you,
> 	ladis


      reply	other threads:[~2017-08-12 11:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-01  8:25 [PATCH 0/2] iio: adc: ti-ads1015: fix channel crosstalk Ladislav Michl
2017-08-01  8:26 ` [PATCH 1/2] iio: adc: ti-ads1015: fix conversion time Ladislav Michl
2017-08-01  9:09   ` Daniel Baluta
2017-08-01  9:59     ` Ladislav Michl
2017-08-01 16:25       ` Daniel Baluta
2017-08-09 14:15   ` Jonathan Cameron
2017-08-09 16:27     ` Ladislav Michl
2017-08-01  8:26 ` [PATCH 2/2] iio: adc: ti-ads1015: use dynamically generated attributes Ladislav Michl
2017-08-09 14:11   ` Jonathan Cameron
2017-08-09 16:44     ` Ladislav Michl
2017-08-12 11:10       ` Jonathan Cameron [this message]

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=20170812121044.034e2c38@archlinux \
    --to=jic23@kernel.org \
    --cc=daniel.baluta@intel.com \
    --cc=ladis@linux-mips.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=matt.ranostay@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).