linux-kernel - Re: [PATCH v14 3/3] Input: new da7280 haptic driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 26 Jun 2020 08:18:53 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Roy Im <roy.im.opensource@...semi.com>
Cc:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Brian Masney <masneyb@...tation.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Lee Jones <lee.jones@...aro.org>, Luca Weiss <luca@...tu.xyz>,
        Maximilian Luz <luzmaximilian@...il.com>,
        Pascal PAILLET-LME <p.paillet@...com>,
        Rob Herring <robh@...nel.org>,
        Samuel Ortiz <sameo@...ux.intel.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Support Opensource <support.opensource@...semi.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pwm@...r.kernel.org
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/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)