| Message ID | 1523731900-4675-15-git-send-email-dinguyen@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | reset: remove request and free functions | expand |
+Stephen for comment Hi Dinh, On 14 April 2018 at 12:51, Dinh Nguyen <dinguyen@kernel.org> wrote: > The request and free reset functions are not really used for any useful > purpose but for debugging. We can safely remove them. The API is set to line up with clocks. I think in general we do want to be able to request and free these devices, just as we do for GPIOs. What is the goal of removing these methods? Regards, Simon
On 04/16/2018 12:43 PM, Simon Glass wrote: > +Stephen for comment > > Hi Dinh, > > On 14 April 2018 at 12:51, Dinh Nguyen <dinguyen@kernel.org> wrote: >> The request and free reset functions are not really used for any useful >> purpose but for debugging. We can safely remove them. > > The API is set to line up with clocks. I think in general we do want > to be able to request and free these devices, just as we do for GPIOs. > What is the goal of removing these methods? Many of the request methods do in fact do something; they check the validity of the reset ID so that check doesn't need to be duplicated everywhere. Even ignoring that, any resource management API should have explicit request/free APIs so that lifetime can be tracked if needed.
On 04/16/2018 01:43 PM, Simon Glass wrote: > +Stephen for comment > > Hi Dinh, > > On 14 April 2018 at 12:51, Dinh Nguyen <dinguyen@kernel.org> wrote: >> The request and free reset functions are not really used for any useful >> purpose but for debugging. We can safely remove them. > > The API is set to line up with clocks. I think in general we do want > to be able to request and free these devices, just as we do for GPIOs. > What is the goal of removing these methods? > It was a suggestion from Marek. I just didn't see any useful code implemented in any of the request/free functions. Dinh
On 04/16/2018 01:51 PM, Stephen Warren wrote: > On 04/16/2018 12:43 PM, Simon Glass wrote: >> +Stephen for comment >> >> Hi Dinh, >> >> On 14 April 2018 at 12:51, Dinh Nguyen <dinguyen@kernel.org> wrote: >>> The request and free reset functions are not really used for any useful >>> purpose but for debugging. We can safely remove them. >> >> The API is set to line up with clocks. I think in general we do want >> to be able to request and free these devices, just as we do for GPIOs. >> What is the goal of removing these methods? > > Many of the request methods do in fact do something; they check the > validity of the reset ID so that check doesn't need to be duplicated > everywhere. Even ignoring that, any resource management API should have > explicit request/free APIs so that lifetime can be tracked if needed. Agreed, that the checks were in some of the request functions, but the majority did not do any checks, just a debug() statement. All of the platforms that did the checks, I just moved them to reset_assert and reset_deassert. Dinh
H Dinh, On 16 April 2018 at 16:41, Dinh Nguyen <dinguyen@kernel.org> wrote: > > > On 04/16/2018 01:51 PM, Stephen Warren wrote: >> On 04/16/2018 12:43 PM, Simon Glass wrote: >>> +Stephen for comment >>> >>> Hi Dinh, >>> >>> On 14 April 2018 at 12:51, Dinh Nguyen <dinguyen@kernel.org> wrote: >>>> The request and free reset functions are not really used for any useful >>>> purpose but for debugging. We can safely remove them. >>> >>> The API is set to line up with clocks. I think in general we do want >>> to be able to request and free these devices, just as we do for GPIOs. >>> What is the goal of removing these methods? >> >> Many of the request methods do in fact do something; they check the >> validity of the reset ID so that check doesn't need to be duplicated >> everywhere. Even ignoring that, any resource management API should have >> explicit request/free APIs so that lifetime can be tracked if needed. > > Agreed, that the checks were in some of the request functions, but the > majority did not do any checks, just a debug() statement. All of the > platforms that did the checks, I just moved them to reset_assert and > reset_deassert. OK. Please can resend the series without removing those methods? Regards, Simon
diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c index 9a5c9c9..24dd48c 100644 --- a/drivers/reset/reset-uclass.c +++ b/drivers/reset/reset-uclass.c @@ -72,12 +72,6 @@ int reset_get_by_index(struct udevice *dev, int index, return ret; } - ret = ops->request(reset_ctl); - if (ret) { - debug("ops->request() failed: %d\n", ret); - return ret; - } - return 0; } @@ -133,24 +127,6 @@ int reset_get_by_name(struct udevice *dev, const char *name, return reset_get_by_index(dev, index, reset_ctl); } -int reset_request(struct reset_ctl *reset_ctl) -{ - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); - - debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); - - return ops->request(reset_ctl); -} - -int reset_free(struct reset_ctl *reset_ctl) -{ - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); - - debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); - - return ops->free(reset_ctl); -} - int reset_assert(struct reset_ctl *reset_ctl) { struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); @@ -209,10 +185,6 @@ int reset_release_all(struct reset_ctl *reset_ctl, int count) ret = reset_assert(&reset_ctl[i]); if (ret) return ret; - - ret = reset_free(&reset_ctl[i]); - if (ret) - return ret; } return 0; diff --git a/include/reset-uclass.h b/include/reset-uclass.h index 50fbeb1..a6301cf 100644 --- a/include/reset-uclass.h +++ b/include/reset-uclass.h @@ -40,27 +40,6 @@ struct reset_ops { int (*of_xlate)(struct reset_ctl *reset_ctl, struct ofnode_phandle_args *args); /** - * request - Request a translated reset control. - * - * The reset core calls this function as the second step in - * implementing a client's reset_get_by_*() call, following a - * successful xxx_xlate() call. - * - * @reset_ctl: The reset control struct to request; this has been - * filled in by a previoux xxx_xlate() function call. - * @return 0 if OK, or a negative error code. - */ - int (*request)(struct reset_ctl *reset_ctl); - /** - * free - Free a previously requested reset control. - * - * This is the implementation of the client reset_free() API. - * - * @reset_ctl: The reset control to free. - * @return 0 if OK, or a negative error code. - */ - int (*free)(struct reset_ctl *reset_ctl); - /** * rst_assert - Assert a reset signal. * * Note: This function is named rst_assert not assert to avoid diff --git a/include/reset.h b/include/reset.h index d38f176..505de77 100644 --- a/include/reset.h +++ b/include/reset.h @@ -134,24 +134,6 @@ int reset_get_by_name(struct udevice *dev, const char *name, struct reset_ctl *reset_ctl); /** - * reset_request - Request a reset signal. - * - * @reset_ctl: A reset control struct. - * - * @return 0 if OK, or a negative error code. - */ -int reset_request(struct reset_ctl *reset_ctl); - -/** - * reset_free - Free a previously requested reset signal. - * - * @reset_ctl: A reset control struct that was previously successfully - * requested by reset_get_by_*(). - * @return 0 if OK, or a negative error code. - */ -int reset_free(struct reset_ctl *reset_ctl); - -/** * reset_assert - Assert a reset signal. * * This function will assert the specified reset signal, thus resetting the @@ -254,11 +236,6 @@ static inline int reset_get_by_name(struct udevice *dev, const char *name, return -ENOTSUPP; } -static inline int reset_free(struct reset_ctl *reset_ctl) -{ - return 0; -} - static inline int reset_assert(struct reset_ctl *reset_ctl) { return 0;
The request and free reset functions are not really used for any useful purpose but for debugging. We can safely remove them. Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- drivers/reset/reset-uclass.c | 28 ---------------------------- include/reset-uclass.h | 21 --------------------- include/reset.h | 23 ----------------------- 3 files changed, 72 deletions(-)