Re: [PATCH V17 3/3] Input: new da7280 haptic driver

[Randy Dunlap]

On 7/23/20 6:54 PM, Roy Im wrote:
> On Fri, July 24, 2020 5:51 AM, Randy Dunlap wrote
>> On 7/23/20 8:01 AM, Roy Im wrote:
>>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>>> index 362e8a0..06dc5a3 100644
>>> --- a/drivers/input/misc/Kconfig
>>> +++ b/drivers/input/misc/Kconfig
>>> @@ -869,4 +869,17 @@ config INPUT_STPMIC1_ONKEY
>>>  	  To compile this driver as a module, choose M here: the
>>>  	  module will be called stpmic1_onkey.
>>>
>>> +config INPUT_DA7280_HAPTICS
>>> +	tristate "Dialog Semiconductor DA7280 haptics support"
>>> +	depends on INPUT && I2C
>>> +	select INPUT_FF_MEMLESS
>>> +	select REGMAP_I2C
>>> +	help
>>> +	  Say Y to enable support for the Dialog DA7280 haptics driver.
>>> +	  The haptics can be controlled by I2C communication,
>>> +	  or by PWM input, or by GPI.
>>
>> 	  Is that                GPIO.
>> ?
> The Haptics can be working by GPI(if see from the haptic device), but from the Host it is GPO. Do you think the GPIO is correct?

To me it needs to represent what services/interfaces/facilities are used by this driver that are
provided by the Linux kernel.  If it uses Linux GPIO services, then it should say GPIO --
although I don't see it using any Linux GPIO services.

>>
>> Can the haptics be controlled only by PWM or only by GPI(O)?
>>
>> Just curious: why is I2C required to build the driver if a user is only controlling the device by PWM or GPI?
> 
> I2C is required to control registers and it can be triggered by I2C or PWM or GPI(controlled by host outside this driver), so PWM and GPI are optional.
> With your comments, I think it's better to remove below lines(//remove) to avoid confusion and add PWM as below if you agree.
>          // remove
> 	  The haptics can be controlled by I2C communication,
> 	  or by PWM input, or by GPI.
>          // update, adding || PWM
>          depends on (INPUT && I2C) || PWM

Since <linux/pwm.h> provides stubs for when CONFIG_PWM is not enabled,
it appears that "depends on  PWM" is not required.

I'll leave it up to you. I was just trying to understand better.
It may be that no changes are needed.


thanks.
-- 
~Randy