Re: [PATCH v1 2/3] drivers: input: misc: Add mtk vibrator driver

[Pavel Machek <pavel@ucw.cz>]

Hi!

> +static int mt_vibra_parse_dt(struct device *dev,
> +		struct reg_vibr_config *vibr_conf)
> +{
> +	int ret;
> +
> +	if (device_property_read_u32(dev, "min-limit",
> +				     &vibr_conf->min_limit))
> +		vibr_conf->min_limit = DEFAULT_MIN_LIMIT;
> +	vibr_conf->min_limit = max_t(unsigned int,
> +		vibr_conf->min_limit, DEFAULT_MIN_LIMIT);
> +
> +	if (device_property_read_u32(dev, "max-limit",
> +				     &vibr_conf->max_limit))
> +		vibr_conf->max_limit = 0;

Is max_limit useful? Do you enforce it in useful way?

> +	if (!vibr_conf->max_limit &&
> +		vibr_conf->max_limit < vibr_conf->min_limit) {
> +		dev_err(dev, "error load dts: get error limitation(min > max)\n");
> +		return -EINVAL;
> +	}

Condition looks wrong.

> +static struct attribute *sysfs_attrs[] = { + &dev_attr_activate.attr, + 
> &dev_attr_state.attr, + &dev_attr_duration.attr, + NULL, +}; + +static struct 
> attribute_group vibr_group = { + .attrs = sysfs_attrs, +}; + +static int 
> regulator_oc_event(struct notifier_block *nb, + unsigned long event, void *data) +{ + 

Do you need custom attributes? Why? Where is their documentation?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html