Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 29 Jun 2023 09:27:03 +0800
From:   ChiYuan Huang <cy_huang@richtek.com>
To:     Mark Brown <broonie@kernel.org>
CC:     <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <robh+dt@kernel.org>, <lgirdwood@gmail.com>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] regulator: rt5739: Add DID check and compatible for
 rt5733
Message-ID: <20230629012703.GA10948@linuxcarl2.richtek.com>
References: <1687942037-14652-1-git-send-email-cy_huang@richtek.com>
 <1687942037-14652-3-git-send-email-cy_huang@richtek.com>
 <2a8732f4-13fe-476d-9ae7-b970bd82c218@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <2a8732f4-13fe-476d-9ae7-b970bd82c218@sirena.org.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Wed, Jun 28, 2023 at 12:47:50PM +0100, Mark Brown wrote:
> On Wed, Jun 28, 2023 at 04:47:17PM +0800, cy_huang@richtek.com wrote:
> 
> > +	if (did == RT5733_CHIPDIE_ID) {
> > +		min_uV = RT5733_VOLT_MINUV;
> > +		max_uV = RT5733_VOLT_MAXUV;
> > +		step_uV = RT5733_VOLT_STPUV;
> > +	} else {
> > +		min_uV = RT5739_VOLT_MINUV;
> > +		max_uV = RT5739_VOLT_MAXUV;
> > +		step_uV = RT5739_VOLT_STPUV;
> > +	}
> 
> It would be better to write these as switch statements so if any more
> variants turn up they can be added more easily.

Since the IC difference is only voltage range and step, They can be retrieved from the
regulator description.

To check DID here may not a good coding.

I may rewrite it as below
  max_uV = desc->min_uV + desc->uV_step * (desc->n_voltages - 1);

And put a switch case for DID check in 'init_regulator_desc'.

Is it better?