Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Philipp,


On 04/04/2017 06:17 PM, Philipp Zabel wrote:
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.

Alright, I will modify the array APIs as suggested to handle an
unordered list of resets passed from the device tree.
As you said, we can handle the special cases when the need arise.


@@ -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.

Thanks

regards
Philipp


Best Regards
Vivek

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux