Re: [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator

From: Fenglin Wu <quic_fenglinw@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<krzysztof.kozlowski+dt@linaro.org>, <robh+dt@kernel.org>,
	<agross@kernel.org>, <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	<linux-input@vger.kernel.org>, <quic_collinsd@quicinc.com>,
	<quic_subbaram@quicinc.com>, <quic_kamalw@quicinc.com>,
	<jestar@qti.qualcomm.com>
Subject: Re: [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
Date: Mon, 25 Sep 2023 10:52:12 +0800	[thread overview]
Message-ID: <36e3719d-ef38-5209-2cd4-7de88257474d@quicinc.com> (raw)
In-Reply-To: <CAA8EJpo7puWxNte5YHiy6=3GdQSeTYCZMe024-b4N0vnxCV0dQ@mail.gmail.com>

On 9/24/2023 3:05 AM, Dmitry Baryshkov wrote:
>> +#define SSBL_VIB_DRV_REG               0x4A
> SSBI_VIB....
> 

Thanks for catching the typo, I will fix it in next patch.

>> +#define SSBI_VIB_DRV_EN_MANUAL_MASK    GENMASK(7, 2)
>> -       /* operate in manual mode */
>> -       error = regmap_read(vib->regmap, regs->drv_addr, &val);
>> -       if (error < 0)
>> -               return error;
>> +       if (data->hw_type != SSBI_VIB) {
> You can drop this condition, if ssbi_vib_data.drv_addr is 0.

I am not sure if I understood this comment: 1st, ssbi_vib_data.drv_addr 
is defined as a constant value 0x4A, so it would never be 0. 2nd, The 
condition check here is to ignore reading the register base address for 
SSBI_VIB HW, so we should do the check based on the HW type.

> 
>> +               error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", &reg_base);
>> +               if (error < 0) {
>> +                       dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
>> +                       return error;
>> +               }
>> +
>> +               vib->reg_base += reg_base;