Messages in this thread | | | Date | Fri, 26 Jun 2020 08:18:53 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v14 3/3] Input: new da7280 haptic driver |
| |
Hello,
from the PWM POV I'm happy now. Just a few minor comments that I noticed while checking the PWM details.
On Thu, Jun 25, 2020 at 01:59:29AM +0900, Roy Im wrote: > + val = haptics->ps_seq_id << DA7280_PS_SEQ_ID_SHIFT | > + haptics->ps_seq_loop << DA7280_PS_SEQ_LOOP_SHIFT;
If you write this as:
val = FIELD_PREP(DA7280_PS_SEQ_ID_MASK, haptics->ps_seq_id) | FIELD_PREP(DA7280_PS_SEQ_LOOP_MASK, haptics->ps_seq_loop);
you get some additional checks for free and can drop all defines for ..._SHIFT .
> +static u8 da7280_haptic_of_gpi_pol_str(struct device *dev, > + const char *str) > +{ > + if (!strcmp(str, "Rising-edge")) > + return 0; > + else if (!strcmp(str, "Falling-edge")) > + return 1; > + else if (!strcmp(str, "Both-edge")) > + return 2; > + > + dev_warn(dev, "Invalid string - set to default\n");
Maybe mention "Rising-edge" being the default?
> + return 0; > +}
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |