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

From: Roy Im
Date: Fri Jul 24 2020 - 01:47:17 EST


Friday, July 24, 2020 11:57 AM, Randy Dunlap wrote
> 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.

OK, let me change to GPIO.

>
> >>
> >> 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.

OK, thanks for your comments. To be clearer, I would like to update as below if you agree

depends on INPUT && I2C
...
The haptics can be controlled by PWM or GPIO
with I2C communication.
>
>
> thanks.
> --
> ~Randy

Kind regards,
Roy