Received-SPF: pass (google.com: domain of linux-kernel+bounces-129057-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <db301b10-6312-4d47-ba4a-9c462e5cbea0@quicinc.com>
Date: Wed, 3 Apr 2024 13:29:46 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 1/3] input: pm8xxx-vibrator: refactor to support new
 SPMI vibrator
Content-Language: en-US
To: Konrad Dybcio <konrad.dybcio@linaro.org>, <kernel@quicinc.com>,
        Andy Gross
	<agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Dmitry Torokhov
	<dmitry.torokhov@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        "Krzysztof
 Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
        Dmitry Baryshkov
	<dmitry.baryshkov@linaro.org>
CC: <linux-arm-msm@vger.kernel.org>, <linux-input@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>
References: <20240401-pm8xxx-vibrator-new-design-v8-0-6f2b8b03b4c7@quicinc.com>
 <20240401-pm8xxx-vibrator-new-design-v8-1-6f2b8b03b4c7@quicinc.com>
 <21641459-d7c0-412d-8244-6f2f2c458551@linaro.org>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
In-Reply-To: <21641459-d7c0-412d-8244-6f2f2c458551@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 4/2/2024 11:21 PM, Konrad Dybcio wrote:
> On 1.04.2024 10:38 AM, Fenglin Wu via B4 Relay wrote:
>> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>>
>> Currently, vibrator control register addresses are hard coded,
>> including the base address and offsets, it's not flexible to
>> support new SPMI vibrator module which is usually included in
>> different PMICs with different base address. Refactor it by using
>> the base address defined in devicetree.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
> 
> [...]
> 
>>   	if (regs->enable_mask)
>> -		rc = regmap_update_bits(vib->regmap, regs->enable_addr,
>> +		rc = regmap_update_bits(vib->regmap, vib->enable_addr,
>>   					regs->enable_mask, on ? ~0 : 0);
> 
> The idiomatic way across the kernel seems to be writing the mask value
> instead of ~0 (which also saves like 2 cpu instructions)
> 
> 
> Not sure about how ssbi addressing works, but except for that lgtm
> 
Agree.
SSBI driver doesn't provide reg_update_bits function call so similar 
mathematics is done on the value before writing to the register,  I can 
update it to use enable_mask directly in next version.

> Konrad