Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <36e3719d-ef38-5209-2cd4-7de88257474d@quicinc.com>
Date:   Mon, 25 Sep 2023 10:52:12 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily
 support new SPMI vibrator
Content-Language: en-US
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>
References: <20230922083801.3056724-1-quic_fenglinw@quicinc.com>
 <20230922083801.3056724-2-quic_fenglinw@quicinc.com>
 <CAA8EJpo7puWxNte5YHiy6=3GdQSeTYCZMe024-b4N0vnxCV0dQ@mail.gmail.com>
From:   Fenglin Wu <quic_fenglinw@quicinc.com>
In-Reply-To: <CAA8EJpo7puWxNte5YHiy6=3GdQSeTYCZMe024-b4N0vnxCV0dQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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;