[PATCH v4 2/8] reset: Add reset controller API

[Stephen Warren]

On 03/04/2013 01:33 AM, Philipp Zabel wrote:
> Hi Stephen,
> 
> Am Freitag, den 01.03.2013, 13:00 -0700 schrieb Stephen Warren:
>> On 02/26/2013 04:39 AM, Philipp Zabel wrote:
>>> This adds a simple API for devices to request being reset
>>> by separate reset controller hardware and implements the
>>> reset signal device tree binding.
>>
>>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>>
>>> +int of_reset_simple_xlate(struct reset_controller_dev *rcdev,
>>> +			  const struct of_phandle_args *reset_spec, u32 *flags)
>>> +{
>>> +	if (WARN_ON(reset_spec->args_count < rcdev->of_reset_n_cells))
>>> +		return -EINVAL;
>>
>> Would != make more sense than < ?
> 
> I copied this from of_gpio_simple_xlate, because the parsing will work
> if args_count > of_reset_n_cells. In this case the device tree contains
> a #reset-cells larger than what the reset controller driver expects.
> Is this reason enough to assume the whole reset_spec is invalid?

Well, there's certainly something unexpected going on; what are those
extra cells expected to mean? I'd tend to make that an error myself to
avoid unexpected symptoms. I'd tend to want to make the same change to
of_gpio_simple_xlate().

>>> +
>>> +	if (reset_spec->args[0] >= rcdev->nr_resets)
>>> +		return -EINVAL;
>>> +
>>> +	if (flags)
>>> +		*flags = reset_spec->args[1];
>>
>> if (flags)
>>     if (reset_spec->args_count > 1)
>>         *flags = reset_spec->args[1];
>>     else
>>         *flags = 0;
>>
>> ?
> 
> In gpiolib, of_get_named_gpio_flags clears the flags, so that the xlate
> implementations don't have to do it. The core doesn't use the flags, so
> this is not a problem. I don't see the need for
> of_get_named_reset_flags, since the reset consumer drivers shouldn't
> care for those. Maybe it would be better to remove the flags parameter
> from the xlate function altogether.

Yes, since there are no flags, removing the parameter makes most sense.