[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f9ad6600-3060-65c5-6f87-8a167c75f8b0@ti.com>
Date: Tue, 4 Jun 2019 10:14:42 -0500
From: Dan Murphy <dmurphy@...com>
To: Mark Brown <broonie@...nel.org>
CC: <jacek.anaszewski@...il.com>, <pavel@....cz>,
<lgirdwood@...il.com>, <lee.jones@...aro.org>,
<linux-leds@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v4 1/6] regulator: lm363x: Make the gpio register
enable flexible
Mark
On 5/30/19 10:26 AM, Mark Brown wrote:
>
> That code is rather far away from the code you're changing and it's
> really not clear that this will do the right thing for new devices, it
> already appears that we're handling two devices without obvious code for
> that (the regulator IDs get reused AFAICT). If there were a switch
> statement right there it'd be clearer.
>
> Part of this is a reviewability thing - I initially stopped on the patch
> because it looked like it was using the enable field for something other
> than the intended purpose and now there's all this chasing around to see
> if the code is OK.
I am going to introduce this update in patch 4 of this series when the
LM36274 is introduced to the regulator driver.
It makes more sense to add it there as in patch 1 there is not really a
need to extend the value or mask as it only supports the LM3632 device.
It makes sense to add the condition when adding another device to support
Thoughts?
Dan
Powered by blists - more mailing lists