devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Mark Brown <broonie@kernel.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org, Ben Dooks <ben-linux@fluff.org>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Russell King <linux@arm.linux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
	Chanwoo Choi <cw00.choi@samsung.com>
Subject: Re: [PATCH 2/8] regulator: max77686: Remove support for board files
Date: Tue, 28 Oct 2014 09:40:50 +0100	[thread overview]
Message-ID: <1414485650.24949.10.camel@AMDC1943> (raw)
In-Reply-To: <20141028003709.GL18557@sirena.org.uk>

On wto, 2014-10-28 at 00:37 +0000, Mark Brown wrote:
> On Mon, Oct 27, 2014 at 04:03:40PM +0100, Krzysztof Kozlowski wrote:
> > The driver is used only on Exynos4 based boards with DTS support.
> > Convert the driver to DTS-only version.
> 
> This doesn't seem like a particularly persuasive reason honestly and if
> you're going to mess around with this stuff please fix it properly...
> 
> >  struct max77686_data {
> > +	struct max77686_regulator_data *regulators;
> > +	int num_regulators;
> > +
> 
> ...the only reason for this array and the specification of the number of
> regulators is to make platform data easier to do...
> 
> > -	pdata->num_regulators = ARRAY_SIZE(regulators);
> > +	max77686->num_regulators = ARRAY_SIZE(regulators);
> 
> ...and indeed we wind up with a constant here anyway.  At the very least
> it'd be better to pull the parsing into the registration, right now we
> have the code still laid out for platform data so it's hard to see it as
> a win in cleanup terms.  If we're not translating into platform data we
> shouldn't need to keep this stuff around outside of the probe function.
> 
> What would be even better would be to convert to use the standard DT
> parsing with regulators_node and of_match specified in the descriptor
> and then delete most of this code entirely.

You're right, there's a lot to clean in this driver.

The only caveat is that I can't test the max77802 regulator so I
refrained from doing intrusive changes to it. I'll try to clean up both
drivers and make max77802 as RFT.

Best regards,
Krzysztof

  reply	other threads:[~2014-10-28  8:40 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-27 15:03 [PATCH 0/8] regulator: max77686: Add GPIO control Krzysztof Kozlowski
2014-10-27 15:03 ` [PATCH 1/8] regulator: max77802: Remove support for board files Krzysztof Kozlowski
2014-10-27 19:36   ` Javier Martinez Canillas
2014-10-28  8:45     ` Krzysztof Kozlowski
2014-10-28  8:50       ` Javier Martinez Canillas
2014-10-27 15:03 ` [PATCH 2/8] regulator: max77686: " Krzysztof Kozlowski
2014-10-27 19:41   ` Javier Martinez Canillas
2014-10-28  0:37   ` Mark Brown
2014-10-28  8:40     ` Krzysztof Kozlowski [this message]
2014-10-27 15:03 ` [PATCH 3/8] mfd: max77686/802: " Krzysztof Kozlowski
2014-10-27 19:48   ` Javier Martinez Canillas
2014-10-28  9:11     ` Krzysztof Kozlowski
2014-10-27 15:03 ` [PATCH 4/8] regulator: max77686: Make regulator_desc array const Krzysztof Kozlowski
2014-10-27 19:50   ` Javier Martinez Canillas
2014-10-28  0:38   ` Mark Brown
2014-10-27 15:03 ` [PATCH 5/8] regulator: max77686: Initialize opmode explicitly to normal mode Krzysztof Kozlowski
2014-10-27 19:51   ` Javier Martinez Canillas
2014-10-27 15:03 ` [PATCH 6/8] regulator: max77686: Add external GPIO control Krzysztof Kozlowski
2014-10-27 20:03   ` Javier Martinez Canillas
2014-10-28  8:52     ` Krzysztof Kozlowski
2014-10-28 12:11       ` Krzysztof Kozlowski
2014-10-29 10:42         ` Krzysztof Kozlowski
2014-10-29 10:49           ` Javier Martinez Canillas
2014-10-30 13:56             ` Alexandre Courbot
2014-10-30 15:03               ` Krzysztof Kozlowski
2014-10-31  3:31                 ` Alexandre Courbot
2014-10-31  7:51                   ` Krzysztof Kozlowski
2014-10-31 10:32                     ` Mark Brown
2014-10-31 11:45                       ` Krzysztof Kozlowski
2014-10-31 11:54                         ` Mark Brown
2014-11-03 12:07                       ` Krzysztof Kozlowski
2014-11-03 13:23                         ` Mark Brown
2014-11-01  5:47                     ` Alexandre Courbot
2014-10-27 15:03 ` [PATCH 7/8] mfd/regulator: dt-bindings: max77686: Document gpio property Krzysztof Kozlowski
2014-10-27 20:15   ` Javier Martinez Canillas
2014-10-28  8:53     ` Krzysztof Kozlowski
2014-10-27 15:03 ` [PATCH 8/8] ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control Krzysztof Kozlowski

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=1414485650.24949.10.camel@AMDC1943 \
    --to=k.kozlowski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=ben-linux@fluff.org \
    --cc=broonie@kernel.org \
    --cc=cw00.choi@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=javier.martinez@collabora.co.uk \
    --cc=kgene.kim@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=sameo@linux.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).