All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: "Jürgen Beisert" <jbe@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org, linux-iio@vger.kernel.org,
	devel@driverdev.osuosl.org, marex@denx.de,
	fabio.estevam@freescale.com, jic23@cam.ac.uk
Subject: Re: [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
Date: Mon, 16 Sep 2013 19:39:40 +0100	[thread overview]
Message-ID: <5237506C.4010607@kernel.org> (raw)
In-Reply-To: <201309161017.31846.jbe@pengutronix.de>

On 09/16/13 09:17, Jürgen Beisert wrote:
> Hi Jonathan,
> 
> On Sunday 15 September 2013 12:35:29 Jonathan Cameron wrote:
>> [...]
>>> +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{
>>> +	writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
>>> +}
>>> +
>>> +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{ 
>>> +	writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
>>> +}
>>> +
>>
>> mxs_lradc_reg_write might be clearer?
> 
> I would prefer the shorter name. Due to long macro names and long function 
> names the lines in the code below needs more and more line breaks which is 
> IMHO less helpfull to read the code.
Hmm.  It's not entirely obvious what else this might be doing, but it is
also not obvious from the name that it is writing to the register...

I remain unconvinced by the short name!


WARNING: multiple messages have this Message-ID (diff)
From: jic23@kernel.org (Jonathan Cameron)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access
Date: Mon, 16 Sep 2013 19:39:40 +0100	[thread overview]
Message-ID: <5237506C.4010607@kernel.org> (raw)
In-Reply-To: <201309161017.31846.jbe@pengutronix.de>

On 09/16/13 09:17, J?rgen Beisert wrote:
> Hi Jonathan,
> 
> On Sunday 15 September 2013 12:35:29 Jonathan Cameron wrote:
>> [...]
>>> +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{
>>> +	writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
>>> +}
>>> +
>>> +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
>>> +{ 
>>> +	writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
>>> +}
>>> +
>>
>> mxs_lradc_reg_write might be clearer?
> 
> I would prefer the shorter name. Due to long macro names and long function 
> names the lines in the code below needs more and more line breaks which is 
> IMHO less helpfull to read the code.
Hmm.  It's not entirely obvious what else this might be doing, but it is
also not obvious from the name that it is writing to the register...

I remain unconvinced by the short name!

  reply	other threads:[~2013-09-16 17:39 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-11  8:18 [RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation Juergen Beisert
2013-09-11  8:18 ` Juergen Beisert
2013-09-11  8:18 ` [PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert
2013-09-11  8:18 ` [PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert
2013-09-15 10:27   ` Jonathan Cameron
2013-09-15 10:27     ` Jonathan Cameron
2013-09-11  8:18 ` [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert
2013-09-15 10:35   ` Jonathan Cameron
2013-09-15 10:35     ` Jonathan Cameron
2013-09-16  8:17     ` Jürgen Beisert
2013-09-16  8:17       ` Jürgen Beisert
2013-09-16 18:39       ` Jonathan Cameron [this message]
2013-09-16 18:39         ` Jonathan Cameron
2013-09-16 14:15   ` Marek Vasut
2013-09-16 14:15     ` Marek Vasut
2013-09-11  8:18 ` [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert
2013-09-15 10:50   ` Jonathan Cameron
2013-09-15 10:50     ` Jonathan Cameron
2013-09-16  8:14     ` Jürgen Beisert
2013-09-16  8:14       ` Jürgen Beisert
2013-09-16 14:19       ` Marek Vasut
2013-09-16 14:19         ` Marek Vasut
2013-09-11  8:18 ` [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert
     [not found]   ` <1378887511-24530-6-git-send-email-jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-09-15 10:56     ` Jonathan Cameron
2013-09-15 10:56       ` Jonathan Cameron
2013-09-15 10:56       ` Jonathan Cameron
     [not found]       ` <52359259.8010202-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-09-15 16:10         ` Jonathan Cameron
2013-09-15 16:10           ` Jonathan Cameron
2013-09-15 16:10           ` Jonathan Cameron
     [not found]           ` <5235DBEA.7030402-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-09-16  8:38             ` Jürgen Beisert
2013-09-16  8:38               ` Jürgen Beisert
2013-09-16  8:38               ` Jürgen Beisert
2013-09-16  8:10         ` Jürgen Beisert
2013-09-16  8:10           ` Jürgen Beisert
2013-09-16  8:10           ` Jürgen Beisert
     [not found]           ` <201309161010.22151.jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-09-16  9:37             ` Jürgen Beisert
2013-09-16  9:37               ` Jürgen Beisert
2013-09-16  9:37               ` Jürgen Beisert
2013-09-16 14:23             ` Marek Vasut
2013-09-16 14:23               ` Marek Vasut
2013-09-16 14:23               ` Marek Vasut
     [not found]               ` <201309161623.48247.marex-ynQEQJNshbs@public.gmane.org>
2013-09-16 14:34                 ` Jürgen Beisert
2013-09-16 14:34                   ` Jürgen Beisert
2013-09-16 14:34                   ` Jürgen Beisert
2013-09-16 15:10                   ` Marek Vasut
2013-09-16 15:10                     ` Marek Vasut
2013-09-16 15:10                     ` Marek Vasut
2013-09-16 15:30             ` Jonathan Cameron
2013-09-16 15:30               ` Jonathan Cameron
2013-09-16 15:30               ` Jonathan Cameron
     [not found]               ` <0fb59809-c835-49b0-bd82-e791db1a9db7-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>
2013-09-19 12:49                 ` Jürgen Beisert
2013-09-19 12:49                   ` Jürgen Beisert
2013-09-19 12:49                   ` Jürgen Beisert
     [not found]                   ` <201309191449.37274.jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-09-19 16:12                     ` Jonathan Cameron
2013-09-19 16:12                       ` Jonathan Cameron
2013-09-19 16:12                       ` Jonathan Cameron
2013-09-16 15:28         ` Dmitry Torokhov
2013-09-16 15:28           ` Dmitry Torokhov
2013-09-16 15:28           ` Dmitry Torokhov
     [not found]           ` <20130916152846.GC12105-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2013-09-17  7:33             ` Jürgen Beisert
2013-09-17  7:33               ` Jürgen Beisert
2013-09-17  7:33               ` Jürgen Beisert
2013-09-11  8:18 ` [PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation Juergen Beisert
2013-09-11  8:18   ` Juergen Beisert

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=5237506C.4010607@kernel.org \
    --to=jic23@kernel.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=fabio.estevam@freescale.com \
    --cc=jbe@pengutronix.de \
    --cc=jic23@cam.ac.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=marex@denx.de \
    /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.