Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets | From | Philipp Zabel <> | Date | Tue, 04 Apr 2017 14:47:49 +0200 |
| |
Hi Vivek,
On Tue, 2017-04-04 at 16:09 +0530, Vivek Gautam wrote: [...] > > I'd prefer to mirror the gpiod API a little, and to have the number > > contained in the array structure, similar to struct gpio_descs: [...] > Alright, i can update this. > I took regulator_bulk interface as the reference, but now i see > gpio_descs has a smaller footprint.
Ok, understood. I think the smaller footprint API is more convenient for the user.
[...] > >> + * Returns 0 on success or error number on failure > >> + */ > >> +static int reset_control_array_get(struct device *dev, int num_rsts, > >> + struct reset_control_array *resets, > >> + bool shared, bool optional) > > Can we make this count and return a pointer to the newly created array? > > > > static struct reset_controls * > > reset_control_array_get(struct device *dev, bool shared, bool optional) > > { > > ... > > > > That way the consumer doesn't have to care about counting and array > > allocation. > > Just trying to think again in line with the regulator bulk APIs. > Can't a user provide a list of reset controls as data and thus > request reset controls with a "id" and num_resets available. > > e.g. > const char * const rst_names[] = { > "rst1", "rst2" ... > }; > xyz_data = { > .resets = rst_names; > .num = ARRAY_SIZE(rst_names); > }; > and then the driver making use of reset_control_array APIs > to request this list of reset controls and assert/deassert them. > > I am assuming this case when one user driver is used across a bunch > of platforms with different number of resets available. > We may not want to rely solely on the device tree entries, since > the resets can be mandatory in few cases and we may want to > return failure.
The way I understood the array API was as a method of handling a list of anonymous resets, specified as
resets = <&reset 1>, <&reset 2>, <&reset 3>; // reset-names = "rst1", "rst2", "rst3"; /* don't care */
in the device tree.
Now if you want to handle a partial list of those as an unordered list, with special consideration for others, that could be added as a separate API when there is use for it. But I expect that most potential users of this array API will not have to make such a distinction.
> >> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get( > >> return -ENOTSUPP; > >> } > >> > >> +static inline int reset_control_array_assert(int num_rsts, > >> + struct reset_control_array *resets) > >> +{ > >> + return 0; > >> +} > >> + > >> +static inline int reset_control_array_deassert(int num_rsts, > >> + struct reset_control_array *resets) > >> +{ > >> + return 0; > >> +} > > Is this missing a stub for reset_control_array_get? > > No, that's supposed to be a static function.
Oh right, it is static. Please ignore.
regards Philipp
| |