usb: Extend ehci and ohci generic drivers
Subsystems (5)
v8 cover usb: Extend ehci and ohci generic drivers
From: Patrice Chotard <[email protected]>
This series improves generic ehci and ohci drivers by addition of :
_ error path during probe (clocks, resets and phy release) _ .remove callback
_ add generic PHY framework for both generic ehci and ohci drivers _ add RESET and CLOCK framework for generic ohci driver
To implement these features, some new methods are needed in reset, clock and in dm/core framework:
_ add reset_request() and reset_assert_all() methods in RESET framework _ add clk_count() and clk_disable_all() methods in CLOCK framework _ add ofnode_count_phandle_with_args() in dm/core
v8: _ rework error path by propagating the initial error code until the end of probe()
_ replace devm_kmalloc() with devm_kcalloc() _ fix cosmetics remarks
v7: _ replace clk_count() and reset_count() methods by
ofnode_count_phandle_with_args() in patches 3, 4 and 5
v6: _ replace clk_get_by_index() by dev_read_phandle_with_args() in
clk_count() in patch 4
_ add Reviewed-by Simon Glass for patch 2 and 5
v5: _ rebase on top of dm/master requested by Simon Glass in order to use
livetree update
_ replace fdtdec_parse_phandle_with_args() by dev_read_phandle_with_args() in patch 2
v4: _ add clk_disable_all() and reset_assert_all() methods into CLOCK and
RESET framework as suggested by Simon Glass and Marek Vasut
_ add reset_count() and clk_count() methods which returns respectively the
number of resets and clocks declared into "resets" and "clocks" DT properties. This allows to allocate the right amount of memory to keep resets and clocks reference
_ update the memory allocation for deasserted resets and enabled
clocks reference list. Replace lists by arrays.
v3: _ keep enabled clocks and deasserted resets reference in list in order to
disable clock or assert resets in error path or in .remove callback
_ add missing commit message
_ use struct generic_ehci * instead of struct udevice * as parameter for
ehci_release_resets() and ehci_release_clocks()
_ test return value on generic_phy_get_by_index() and
generic_phy_init()
_ split previous patch 5 in 3 independant patch for CLOCK, RESET and PHY support
v2: _ add needed reset_request() in RESET framework
_ add error path in ehci/ohci-generic to disable clocks and to assert resets
_ add .remove callback with clocks, resets and phy release _ split the replacement of printf() by error() in an independant patch
Patrice Chotard (10):
reset: add reset_request()
reset: add reset_assert_all()
clk: add clk_disable_all()
dm: core: add ofnode_count_phandle_with_args()
usb: host: ehci-generic: replace printf() by error()
usb: host: ehci-generic: add error path and .remove callback
usb: host: ehci-generic: add generic PHY support
usb: host: ohci-generic: add CLOCK support
usb: host: ohci-generic: add RESET support
usb: host: ohci-generic: add generic PHY support
drivers/clk/clk-uclass.c | 22 ++++++ drivers/core/of_access.c | 7 ++ drivers/core/ofnode.c | 12 ++++ drivers/reset/reset-uclass.c | 31 +++++++++ drivers/usb/host/ehci-generic.c | 149 +++++++++++++++++++++++++++++++++------- drivers/usb/host/ohci-generic.c | 130 ++++++++++++++++++++++++++++++++++- include/clk.h | 10 +++ include/dm/of_access.h | 18 +++++ include/dm/ofnode.h | 17 +++++ include/reset.h | 26 +++++++ 10 files changed, 397 insertions(+), 25 deletions(-)
Signature · 1 line
1.9.1
PATCH v8 1/10 reset: add reset_request()
From: Patrice Chotard <[email protected]>
This is needed in error path to assert previously deasserted reset by using a saved reset_ctl reference.
---
v8: _ none
v7: _ none
v6: _ none
v5: _ none
v4: _ none
v3: _ none
v2: _ none
drivers/reset/reset-uclass.c | 9 +++++++++ include/reset.h | 9 +++++++++ 2 files changed, 18 insertions(+)
drivers/reset/reset-uclass.c +9 -0
@@ -97,6 +97,15 @@ 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);
include/reset.h +9 -0
@@ -100,6 +100,15 @@ 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
Signature · 1 line
1.9.1
PATCH v8 2/10 reset: add reset_assert_all()
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
Add reset_assert_all() method which Request/Assert/Free an array of resets signal that has been previously successfully requested by reset_get_by_*()
---
v8: _ none
v7: _ none
v6: _ none
v5: _ none
v4: _ add reset_assert_all() method as suggested by Marek Vasut
and Simon Glass
drivers/reset/reset-uclass.c | 22 ++++++++++++++++++++++ include/reset.h | 17 +++++++++++++++++ 2 files changed, 39 insertions(+)
drivers/reset/reset-uclass.c +22 -0
@@ -133,6 +133,28 @@ int reset_deassert(struct reset_ctl *reset_ctl)return ops->rst_deassert (reset_ctl); } +int reset_assert_all (struct reset_ctl *reset_ctl,int count) +{ +int i, ret; + +for (i =0 ; i < count; i++) { +debug ("%s(reset_ctl[%d]=%p)\n" , __func__, i, &reset_ctl[i]); + + ret =reset_request (&reset_ctl[i]); +if (ret) +return ret; + + ret =reset_assert (&reset_ctl[i]); +if (ret) +return ret; + + ret =reset_free (&reset_ctl[i]); +if (ret) +return ret; + } +return 0 ; +} +UCLASS_DRIVER (reset) = { .id =UCLASS_RESET , .name ="reset" ,
include/reset.h +17 -0
@@ -144,6 +144,17 @@ int reset_assert(struct reset_ctl *reset_ctl);*/ int reset_deassert (struct reset_ctl *reset_ctl); +/** +* reset_assert_all - Request/Assert/Free resets. +* +* This function will request, assert and free array of clocks, +* +* @reset_ctl: A reset struct array that was previously successfully +* requested by reset_get_by_*(). +* @count Number of reset contained in the array +* @return 0 if OK, or a negative error code. +*/ +int reset_assert_all (struct reset_ctl *reset_ctl,int count);#else static inline int reset_get_by_index (struct udevice *dev,int index,struct reset_ctl *reset_ctl)
@@ -171,6 +182,12 @@ static inline int reset_deassert(struct reset_ctl *reset_ctl) {return 0 ; } + +static inline int reset_assert_all (struct reset_ctl *reset_ctl,int count) +{ +return 0 ; +} +#endif #endif
Signature · 1 line
1.9.1
PATCH v8 2/10 reset: add reset_assert_all()
Hi,
Quoted 6 lines · On Wed, 21 Jun 2017 09:50:16 +0200 patrice.chotard at st.com wrote:
From: Patrice Chotard <[email protected]>
Add
reset_assert_all()method which Request/Assert/Free an array of resets signal that has been previously successfully requested by reset_get_by_*()
IMO this is a terrible API. The purpose of a request() function is to mark a resource as in-use, so that it cannot be deallocated or otherwise destroyed until it is explicitly freed by the user.
It doesn't make any sense to request a resource, perform an action on it (enable/disable a clock, assert/deasser a reset line, ...) and free it right away.
The caller of reset_assert_all() should make sure that all reset resources are requested before calling this function and keep them requested until after relinquish using them.
The same holds for the clk_disable_all() in your other patch.
Lothar Waßmann
PATCH v8 2/10 reset: add reset_assert_all()
Hi
Quoted 13 lines · Lothar Waßmann
Hi,
Quoted 6 lines · On Wed, 21 Jun 2017 09:50:16 +0200 patrice.chotard at st.com wrote:
From: Patrice Chotard <[email protected]>
Add
reset_assert_all()method which Request/Assert/Free an array of resets signal that has been previously successfully requested by reset_get_by_*()IMO this is a terrible API. The purpose of a
request()function is to mark a resource as in-use, so that it cannot be deallocated or otherwise destroyed until it is explicitly freed by the user.
Quoted 8 lines
It doesn't make any sense to request a resource, perform an action on it (enable/disable a clock, assert/deasser a reset line, ...) and free it right away.
The caller of
reset_assert_all()should make sure that all reset resources are requested before calling this function and keep them requested until after relinquish using them.
The same holds for theclk_disable_all()in your other patch.
Following your remark in patch 7, i have started to rework this API as you described here by removing the request in reset_assert_all() and in clock_disable_all(). I agree it makes no sense.
It will be fixed in the v9
Thanks
Patrice
Quoted 5 lines
Lothar Waßmann
PATCH v8 3/10 clk: add clk_disable_all()
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
Add clk_disable_all() method which Request/Disable/Free an array of clocks that has been previously requested by clk_request/get_by_*()
---
v8: _ replace clk->dev by clk[i].dev in clk_request() param
v7: _ none
v6: _ none
v5: _ none
v4: _ none
v3: _ add commit message
v2: _ create this independant path for printf() replacement
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 10 ++++++++++ 2 files changed, 32 insertions(+)
drivers/clk/clk-uclass.c +22 -0
@@ -187,6 +187,28 @@ int clk_disable(struct clk *clk)return ops->disable (clk); } +int clk_disable_all (struct clk *clk,int count) +{ +int i, ret; + +debug ("%s(clk=%p count=%d)\n" , __func__, clk, count); + +for (i =0 ; i < count; i++) { + ret =clk_request (clk[i].dev, &clk[i]); +if (ret && ret != -ENOSYS ) +return ret; + + ret =clk_disable (&clk[i]); +if (ret && ret != -ENOSYS ) +return ret; + + ret =clk_free (&clk[i]); +if (ret && ret != -ENOSYS ) +return ret; + } +return 0 ; +} +UCLASS_DRIVER (clk) = { .id =UCLASS_CLK , .name ="clk" ,
include/clk.h +10 -0
@@ -174,6 +174,16 @@ int clk_enable(struct clk *clk);*/ int clk_disable (struct clk *clk); +/** +* clk_disable_all() - Request/Disable (turn off)/Free clocks. +* +* @clk: A clock struct array that was previously successfully +* requested by clk_request/get_by_*(). +* @count Number of clock contained in the array +* @return zero on success, or -ve error code. +*/ +int clk_disable_all (struct clk *clk,int count); +int soc_clk_dump (void );#endif
Signature · 1 line
1.9.1
PATCH v8 4/10 dm: core: add ofnode_count_phandle_with_args()
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
This function is usefull to get phandle number contained in a property list.
For example, this allows to allocate the right amount of memory to keep clock's reference contained into the "clocks" property.
To implement it, either of_count_phandle_with_args() or fdtdec_parse_phandle_with_args() are used respectively for live tree and flat tree.
By passing index = -1, these 2 functions returns the number of phandle contained into the property list.
---
v8: _ none
v7: _ add ofnode_count_phandle_with_args() which returns
the phandle number contained into a property list.
drivers/core/of_access.c | 7 +++++++ drivers/core/ofnode.c | 12 ++++++++++++ include/dm/of_access.h | 18 ++++++++++++++++++ include/dm/ofnode.h | 17 +++++++++++++++++ 4 files changed, 54 insertions(+)
drivers/core/of_access.c +7 -0
@@ -641,6 +641,13 @@ int of_parse_phandle_with_args(const struct device_node *np, index, out_args); } +int of_count_phandle_with_args (const struct device_node *np, +const char *list_name,const char *cells_name) +{ +return __of_parse_phandle_with_args (np, list_name, cells_name,0 , +-1 ,NULL ); +} +static void of_alias_add (struct alias_prop *ap,struct device_node *np,int id,const char *stem,int stem_len) {
drivers/core/ofnode.c +12 -0
@@ -307,6 +307,18 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name,return 0 ; } +int ofnode_count_phandle_with_args (ofnode node,const char *list_name, +const char *cells_name) +{ +if (ofnode_is_np (node)) +return of_count_phandle_with_args (ofnode_to_np (node), + list_name, cells_name); +else +return fdtdec_parse_phandle_with_args (gd->fdt_blob, +ofnode_to_offset (node), list_name, cells_name, +0 ,-1 ,NULL ); +} +ofnode ofnode_path (const char *path) {if (of_live_active ())
include/dm/of_access.h +18 -0
@@ -315,6 +315,24 @@ int of_parse_phandle_with_args(const struct device_node *np, int index, struct of_phandle_args *out_args);/** +* of_count_phandle_with_args() - Count the number of phandle in a list +* +* @np: pointer to a device tree node containing a list +* @list_name: property name that contains a list +* @cells_name: property name that specifies phandles' arguments count +* @return number of phandle found, -ENOENT if +* @list_name does not exist, -EINVAL if a phandle was not found, +* @cells_name could not be found, the arguments were truncated or there +* were too many arguments. +* +* Returns number of phandle found on success, on error returns appropriate +* errno value. +* +*/ +int of_count_phandle_with_args (const struct device_node *np, +const char *list_name,const char *cells_name); + +/** * of_alias_scan() - Scan all properties of the 'aliases' node * * The function scans all the properties of the 'aliases' node and populates
include/dm/ofnode.h +17 -0
@@ -423,6 +423,23 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name, struct ofnode_phandle_args *out_args);/** +* ofnode_count_phandle_with_args() - Count number of phandle in a list +* +* This function is useful to count phandles into a list. +* Returns number of phandle on success, on error returns appropriate +* errno value. +* +* @node: device tree node containing a list +* @list_name: property name that contains a list +* @cells_name: property name that specifies phandles' arguments count +* @return number of phandle on success, -ENOENT if @list_name does not +* exist, -EINVAL if a phandle was not found, @cells_name could not +* be found. +*/ +int ofnode_count_phandle_with_args (ofnode node,const char *list_name, +const char *cells_name); + +/** * ofnode_path() - find a node by full path * * @path: Full path to node, e.g. "/bus/spi at 1"
Signature · 1 line
1.9.1
PATCH v8 5/10 usb: host: ehci-generic: replace printf() by error()
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
this allows to get file, line and function location of the current error message.
---
v8: _ none
v7: _ none
v6: _ none
v5: _ none
v4: _ none
v3: _ add commit message
v2: _ create this independant path for printf() replacement
drivers/usb/host/ehci-generic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-generic.c +2 -2
@@ -34,7 +34,7 @@ static int ehci_usb_probe(struct udevice *dev)if (ret <0 )break ;if (clk_enable (&clk)) -printf ("failed to enable clock %d\n" , i); +error ("failed to enable clock %d\n" , i);clk_free (&clk); }
@@ -46,7 +46,7 @@ static int ehci_usb_probe(struct udevice *dev)if (ret <0 )break ;if (reset_deassert (&reset)) -printf ("failed to deassert reset %d\n" , i); +error ("failed to deassert reset %d\n" , i);reset_free (&reset); }
Signature · 1 line
1.9.1
PATCH v8 6/10 usb: host: ehci-generic: add error path and .remove callback
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
Use an array to save enabled clocks reference and deasserted resets in order to respectively disabled and asserted them in case of error during probe() or during driver removal.
---
v8: _ replace devm_kmalloc() by devm_kcalloc()
_ fix error path by propagating initial error code until the end of probe()
v7: _ replace clk_count() and reset_count() by ofnode_count_phandle_with_args()
v6: _ none
v5: _ none
v4: _ update the memory allocation for deasserted resets and enabled
clocks reference list. Replace lists by arrays.
_ usage of new RESET and CLOCK methods clk_count(), reset_count(),
reset_assert_all() and clk_disable_all().
v3: _ keep enabled clocks and deasserted resets reference in list in order to
disable clock or assert resets in error path or in .remove callback
_ use struct generic_ehci * instead of struct udevice * as parameter for
ehci_release_resets() and ehci_release_clocks()
drivers/usb/host/ehci-generic.c | 120 ++++++++++++++++++++++++++++++++-------- 1 file changed, 97 insertions(+), 23 deletions(-)
drivers/usb/host/ehci-generic.c +97 -23
@@ -6,6 +6,7 @@#include <common.h> #include <clk.h> +#include <dm/ofnode.h> #include <reset.h> #include <asm/io.h> #include <dm.h>
@@ -18,43 +19,116 @@*/ struct generic_ehci {struct ehci_ctrl ctrl; +struct clk *clocks; +struct reset_ctl *resets; +int clock_count; +int reset_count; };static int ehci_usb_probe (struct udevice *dev) { +struct generic_ehci *priv =dev_get_priv (dev);struct ehci_hccr *hccr;struct ehci_hcor *hcor; -int i; - -for (i =0 ; ; i++) { -struct clk clk; -int ret; - - ret =clk_get_by_index (dev, i, &clk); -if (ret <0 ) -break ; -if (clk_enable (&clk)) -error ("failed to enable clock %d\n" , i); -clk_free (&clk); +int i, err, ret, clock_nb, reset_nb; + + err =0 ; + priv->clock_count =0 ; + clock_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"clocks" , +"#clock-cells" ); +if (clock_nb >0 ) { + priv->clocks =devm_kcalloc (dev, clock_nb,sizeof (struct clk ), +GFP_KERNEL ); +if (!priv->clocks) +return -ENOMEM ; + +for (i =0 ; i < clock_nb; i++) { + err =clk_get_by_index (dev, i, &priv->clocks[i]); + +if (err <0 ) +break ; + err =clk_enable (&priv->clocks[i]); +if (err) { +error ("failed to enable clock %d\n" , i); +clk_free (&priv->clocks[i]); + goto clk_err; + } + priv->clock_count++; +clk_free (&priv->clocks[i]); + } + }else { +if (clock_nb != -ENOENT ) { +error ("failed to get clock phandle(%d)\n" , clock_nb); +return clock_nb; + } } -for (i =0 ; ; i++) { -struct reset_ctl reset; -int ret; + priv->reset_count =0 ; + reset_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"resets" , +"#reset-cells" ); +if (reset_nb >0 ) { + priv->resets =devm_kcalloc (dev, reset_nb, +sizeof (struct reset_ctl ), +GFP_KERNEL ); +if (!priv->resets) +return -ENOMEM ; + +for (i =0 ; i < reset_nb; i++) { + err =reset_get_by_index (dev, i, &priv->resets[i]); +if (err <0 ) +break ; - ret =reset_get_by_index (dev, i, &reset); -if (ret <0 ) -break ; -if (reset_deassert (&reset)) -error ("failed to deassert reset %d\n" , i); -reset_free (&reset); +if (reset_deassert (&priv->resets[i])) { +error ("failed to deassert reset %d\n" , i); +reset_free (&priv->resets[i]); + goto reset_err; + } + priv->reset_count++; +reset_free (&priv->resets[i]); + } + }else { +if (reset_nb != -ENOENT ) { +error ("failed to get reset phandle(%d)\n" , reset_nb); + goto clk_err; + } } hccr =map_physmem (devfdt_get_addr (dev),0x100 ,MAP_NOCACHE ); hcor = (struct ehci_hcor *)((uintptr_t )hccr +HC_LENGTH (ehci_readl (&hccr->cr_capbase))); -return ehci_register (dev, hccr, hcor,NULL ,0 ,USB_INIT_HOST ); + err =ehci_register (dev, hccr, hcor,NULL ,0 ,USB_INIT_HOST ); +if (err) + goto reset_err; + +return 0 ; + +reset_err: + ret =reset_assert_all (priv->resets, priv->reset_count); +if (ret) +error ("failed to assert all resets\n" ); +clk_err: + ret =clk_disable_all (priv->clocks, priv->clock_count); +if (ret) +error ("failed to disable all clocks\n" ); + +return err; +} + +static int ehci_usb_remove (struct udevice *dev) +{ +struct generic_ehci *priv =dev_get_priv (dev); +int ret; + + ret =ehci_deregister (dev); +if (ret) +return ret; + + ret =reset_assert_all (priv->resets, priv->reset_count); +if (ret) +return ret; + +return clk_disable_all (priv->clocks, priv->clock_count); }static const struct udevice_id ehci_usb_ids[] = {
@@ -67,7 +141,7 @@ U_BOOT_DRIVER(ehci_generic) = { .id = UCLASS_USB, .of_match = ehci_usb_ids, .probe = ehci_usb_probe, - .remove = ehci_deregister, + .remove = ehci_usb_remove, .ops = &ehci_usb_ops, .priv_auto_alloc_size = sizeof(struct generic_ehci), .flags = DM_FLAG_ALLOC_PRIV_DMA,
Signature · 1 line
1.9.1
PATCH v8 7/10 usb: host: ehci-generic: add generic PHY support
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
Extend ehci-generic driver with generic PHY framework
---
v8: _ rework error path by propagating the initial error code until the end of probe()
v7: _ none
v6: _ none
v5: _ none
v4: _ update the memory allocation for deasserted resets and enabled
clocks reference list. Replace lists by arrays.
_ usage of new RESET and CLOCK methods clk_count(), reset_count(),
reset_assert_all() and clk_disable_all().
v3: _ keep enabled clocks and deasserted resets reference in list in order to
disable clock or assert resets in error path or in .remove callback
_ use struct generic_ehci * instead of struct udevice * as parameter for
ehci_release_resets() and ehci_release_clocks()
drivers/usb/host/ehci-generic.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
drivers/usb/host/ehci-generic.c +30 -1
@@ -7,6 +7,7 @@#include <common.h> #include <clk.h> #include <dm/ofnode.h> +#include <generic-phy.h> #include <reset.h> #include <asm/io.h> #include <dm.h>
@@ -21,6 +22,7 @@ struct generic_ehci {struct ehci_ctrl ctrl;struct clk *clocks;struct reset_ctl *resets; +struct phy phy;int clock_count;int reset_count; };
@@ -93,16 +95,37 @@ static int ehci_usb_probe(struct udevice *dev) } } + err =generic_phy_get_by_index (dev,0 , &priv->phy); +if (err) { +if (err != -ENOENT ) { +error ("failed to get usb phy\n" ); + goto reset_err; + } + } + + err =generic_phy_init (&priv->phy); +if (err) { +error ("failed to init usb phy\n" ); + goto reset_err; + } + hccr =map_physmem (devfdt_get_addr (dev),0x100 ,MAP_NOCACHE ); hcor = (struct ehci_hcor *)((uintptr_t )hccr +HC_LENGTH (ehci_readl (&hccr->cr_capbase))); err =ehci_register (dev, hccr, hcor,NULL ,0 ,USB_INIT_HOST );if (err) - goto reset_err; + goto phy_err;return 0 ; +phy_err: +if (generic_phy_valid (&priv->phy)) { + ret =generic_phy_exit (&priv->phy); +if (ret) +error ("failed to release phy\n" ); + } + reset_err: ret =reset_assert_all (priv->resets, priv->reset_count);if (ret)
@@ -124,6 +147,12 @@ static int ehci_usb_remove(struct udevice *dev)if (ret)return ret; +if (generic_phy_valid (&priv->phy)) { + ret =generic_phy_exit (&priv->phy); +if (ret) +return ret; + } + ret =reset_assert_all (priv->resets, priv->reset_count);if (ret)return ret;
Signature · 1 line
1.9.1
PATCH v8 8/10 usb: host: ohci-generic: add CLOCK support
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
use array to save enabled clocks reference in order to disabled them in case of error during probe() or during driver removal.
---
v8: _ rework error path by propagating the initial error code until the end of probe()
v7: _ replace clk_count() by ofnode_count_phandle_with_args()
v6: _ none
v5: _ none
v4: _ use generic_phy_valid() before generic_phy_exit() call
v3: _ extract in this patch the CLOCK support add-on from previous patch 5
_ keep enabled clocks reference in list in order to
disable clocks in error path or in .remove callback
v2: _ add error path management
_ add .remove callback
drivers/usb/host/ohci-generic.c | 59 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-)
drivers/usb/host/ohci-generic.c +57 -2
@@ -5,7 +5,9 @@*/ #include <common.h> +#include <clk.h> #include <dm.h> +#include <dm/ofnode.h> #include "ohci.h" #if !defined(CONFIG_USB_OHCI_NEW )
@@ -14,18 +16,71 @@struct generic_ohci {ohci_t ohci; +struct clk *clocks; +int clock_count; };static int ohci_usb_probe (struct udevice *dev) {struct ohci_regs *regs = (struct ohci_regs *)devfdt_get_addr (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int i, err, ret, clock_nb; -return ohci_register (dev, regs); + err =0 ; + priv->clock_count =0 ; + clock_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"clocks" , +"#clock-cells" ); +if (clock_nb >0 ) { + priv->clocks =devm_kcalloc (dev, clock_nb,sizeof (struct clk ), +GFP_KERNEL ); +if (!priv->clocks) +return -ENOMEM ; + +for (i =0 ; i < clock_nb; i++) { + err =clk_get_by_index (dev, i, &priv->clocks[i]); +if (err <0 ) +break ; + + err =clk_enable (&priv->clocks[i]); +if (err) { +error ("failed to enable clock %d\n" , i); +clk_free (&priv->clocks[i]); + goto clk_err; + } + priv->clock_count++; +clk_free (&priv->clocks[i]); + } + }else { +if (clock_nb != -ENOENT ) { +error ("failed to get clock phandle(%d)\n" , clock_nb); +return clock_nb; + } + } + + err =ohci_register (dev, regs); +if (err) + goto clk_err; + +return 0 ; + +clk_err: + ret =clk_disable_all (priv->clocks, priv->clock_count); +if (ret) +error ("failed to disable all clocks\n" ); + +return err; }static int ohci_usb_remove (struct udevice *dev) { -return ohci_deregister (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int ret; + + ret =ohci_deregister (dev); +if (ret) +return ret; + +return clk_disable_all (priv->clocks, priv->clock_count); }static const struct udevice_id ohci_usb_ids[] = {
Signature · 1 line
1.9.1
PATCH v8 8/10 usb: host: ohci-generic: add CLOCK support
Quoted 22 lines · On 21 June 2017 at 01:50, <[email protected]> wrote:
From: Patrice Chotard <[email protected]>
use array to save enabled clocks reference in order to disabled them in case of error during
probe()or during driver removal.Signed-off-by Patrice Chotard <[email protected]>---
v8: _ rework error path by propagating the initial error code until the end ofprobe()v7: _ replace
clk_count()byofnode_count_phandle_with_args()v6: _ none
v5: _ none
v4: _ use
generic_phy_valid()beforegeneric_phy_exit()callv3: _ extract in this patch the CLOCK support add-on from previous patch 5
_ keep enabled clocks reference in list in order to
disable clocks in error path or in .remove callback
Quoted 5 lines
v2: _ add error path management
_ add .remove callbackdrivers/usb/host/ohci-generic.c | 59 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-)
Nits below (sorry) for a respin or follow-on.
Quoted 16 lines
drivers/usb/host/ohci-generic.c +2
-0@@ -5,7 +5,9 @@*/ #include <common.h> +#include <clk.h> #include <dm.h> +#include <dm/ofnode.h> #include "ohci.h" #if !defined(CONFIG_USB_OHCI_NEW )
drivers/usb/host/ohci-generic.c line 21
Quoted 6 lines
drivers/usb/host/ohci-generic.c +2
-0@@ -14,18 +16,71 @@struct generic_ohci {ohci_t ohci; +struct clk *clocks; +int clock_count; };
Please can you comment this struct?
drivers/usb/host/ohci-generic.c line 32
Quoted 12 lines
drivers/usb/host/ohci-generic.c +6
-1static int ohci_usb_probe (struct udevice *dev) {struct ohci_regs *regs = (struct ohci_regs *)devfdt_get_addr (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int i, err, ret, clock_nb; -return ohci_register (dev, regs); + err =0 ; + priv->clock_count =0 ; + clock_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"clocks" , +"#clock-cells" );
Could we have a dev_read_...() version of this?
Quoted 12 lines
drivers/usb/host/ohci-generic.c +12
-0+if (clock_nb >0 ) { + priv->clocks =devm_kcalloc (dev, clock_nb,sizeof (struct clk ), +GFP_KERNEL ); +if (!priv->clocks) +return -ENOMEM ; + +for (i =0 ; i < clock_nb; i++) { + err =clk_get_by_index (dev, i, &priv->clocks[i]); +if (err <0 ) +break ; + + err =clk_enable (&priv->clocks[i]);
drivers/usb/host/ohci-generic.c line 54
Quoted 10 lines
drivers/usb/host/ohci-generic.c +10
-0+if (err) { +error ("failed to enable clock %d\n" , i); +clk_free (&priv->clocks[i]); + goto clk_err; + } + priv->clock_count++; +clk_free (&priv->clocks[i]); + } + }else { +if (clock_nb != -ENOENT ) {
} else if (...
?
Quoted 34 lines
drivers/usb/host/ohci-generic.c +25
-2+error ("failed to get clock phandle(%d)\n" , clock_nb); +return clock_nb; + } + } + + err =ohci_register (dev, regs); +if (err) + goto clk_err; + +return 0 ; + +clk_err: + ret =clk_disable_all (priv->clocks, priv->clock_count); +if (ret) +error ("failed to disable all clocks\n" ); + +return err; }static int ohci_usb_remove (struct udevice *dev) { -return ohci_deregister (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int ret; + + ret =ohci_deregister (dev); +if (ret) +return ret; + +return clk_disable_all (priv->clocks, priv->clock_count); }static const struct udevice_id ohci_usb_ids[] = { --
Quoted 2 lines
1.9.1
Regards,
Simon
PATCH v8 8/10 usb: host: ohci-generic: add CLOCK support
Hi Simon
Quoted 65 lines · Simon Glass
Quoted 22 lines · On 21 June 2017 at 01:50, <[email protected]> wrote:
From: Patrice Chotard <[email protected]>
use array to save enabled clocks reference in order to disabled them in case of error during
probe()or during driver removal.Signed-off-by Patrice Chotard <[email protected]>---
v8: _ rework error path by propagating the initial error code until the end ofprobe()v7: _ replace
clk_count()byofnode_count_phandle_with_args()v6: _ none
v5: _ none
v4: _ use
generic_phy_valid()beforegeneric_phy_exit()callv3: _ extract in this patch the CLOCK support add-on from previous patch 5
_ keep enabled clocks reference in list in order to
disable clocks in error path or in .remove callbackQuoted 5 lines
v2: _ add error path management
_ add .remove callbackdrivers/usb/host/ohci-generic.c | 59 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-)Reviewed-by Simon Glass <[email protected]>Nits below (sorry) for a respin or follow-on.
Quoted 16 lines
drivers/usb/host/ohci-generic.c +2
-0@@ -5,7 +5,9 @@*/ #include <common.h> +#include <clk.h> #include <dm.h> +#include <dm/ofnode.h> #include "ohci.h" #if !defined(CONFIG_USB_OHCI_NEW )drivers/usb/host/ohci-generic.c line 21
Quoted 6 lines
drivers/usb/host/ohci-generic.c +2
-0@@ -14,18 +16,71 @@struct generic_ohci {ohci_t ohci; +struct clk *clocks; +int clock_count; };Please can you comment this struct?
Quoted 7 lines
drivers/usb/host/ohci-generic.c +2
-0static int ohci_usb_probe (struct udevice *dev) {struct ohci_regs *regs = (struct ohci_regs *)devfdt_get_addr (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int i, err, ret, clock_nb;
Quoted 7 lines
drivers/usb/host/ohci-generic.c line 32
Quoted 5 lines
drivers/usb/host/ohci-generic.c +4
-1-return ohci_register (dev, regs); + err =0 ; + priv->clock_count =0 ; + clock_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"clocks" , +"#clock-cells" );Could we have a dev_read_...() version of this?
Yes, i will add it
Quoted 17 lines
Quoted 17 lines
drivers/usb/host/ohci-generic.c +16
-0+if (clock_nb >0 ) { + priv->clocks =devm_kcalloc (dev, clock_nb,sizeof (struct clk ), +GFP_KERNEL ); +if (!priv->clocks) +return -ENOMEM ; + +for (i =0 ; i < clock_nb; i++) { + err =clk_get_by_index (dev, i, &priv->clocks[i]); +if (err <0 ) +break ; + + err =clk_enable (&priv->clocks[i]); +if (err) { +error ("failed to enable clock %d\n" , i); +clk_free (&priv->clocks[i]); + goto clk_err;
Quoted 8 lines
drivers/usb/host/ohci-generic.c line 54
Quoted 6 lines
drivers/usb/host/ohci-generic.c +6
-0+ } + priv->clock_count++; +clk_free (&priv->clocks[i]); + } + }else { +if (clock_nb != -ENOENT ) {} else if (...
ok
Quoted 43 lines
?
Quoted 34 lines
drivers/usb/host/ohci-generic.c +25
-2+error ("failed to get clock phandle(%d)\n" , clock_nb); +return clock_nb; + } + } + + err =ohci_register (dev, regs); +if (err) + goto clk_err; + +return 0 ; + +clk_err: + ret =clk_disable_all (priv->clocks, priv->clock_count); +if (ret) +error ("failed to disable all clocks\n" ); + +return err; }static int ohci_usb_remove (struct udevice *dev) { -return ohci_deregister (dev); +struct generic_ohci *priv =dev_get_priv (dev); +int ret; + + ret =ohci_deregister (dev); +if (ret) +return ret; + +return clk_disable_all (priv->clocks, priv->clock_count); }static const struct udevice_id ohci_usb_ids[] = { --Quoted 2 lines
1.9.1
Regards,
Simon
PATCH v8 9/10 usb: host: ohci-generic: add RESET support
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
use array to save deasserted resets reference in order to assert them in case of error during probe() or during driver removal.
---
v8: _ rework error path by propagating the initial error code until the end of probe()
_ replace devm_kmalloc() by devm_kcalloc()
v7: _ replace reset_count() by ofnode_count_phandle_with_args()
v6: _ none
v5: _ none
v4: _ update the memory allocation for deasserted resets. Replace lists by arrays.
_ usage of new RESET methods reset_assert_all() and clk_disable_all().
v3: _ extract in this patch the RESET support add-on from previous patch 5
_ keep deasserted resets reference in list in order to
assert resets in error path or in .remove callback
v2: _ add error path management
_ add .remove callback
drivers/usb/host/ohci-generic.c | 46 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-)
drivers/usb/host/ohci-generic.c +44 -2
@@ -8,6 +8,7 @@#include <clk.h> #include <dm.h> #include <dm/ofnode.h> +#include <reset.h> #include "ohci.h" #if !defined(CONFIG_USB_OHCI_NEW )
@@ -17,14 +18,16 @@struct generic_ohci {ohci_t ohci;struct clk *clocks; +struct reset_ctl *resets;int clock_count; +int reset_count; };static int ohci_usb_probe (struct udevice *dev) {struct ohci_regs *regs = (struct ohci_regs *)devfdt_get_addr (dev);struct generic_ohci *priv =dev_get_priv (dev); -int i, err, ret, clock_nb; +int i, err, ret, clock_nb, reset_nb; err =0 ; priv->clock_count =0 ;
@@ -57,12 +60,47 @@ static int ohci_usb_probe(struct udevice *dev) } } + priv->reset_count =0 ; + reset_nb =ofnode_count_phandle_with_args (dev_ofnode (dev),"resets" , +"#reset-cells" ); +if (reset_nb >0 ) { + priv->resets =devm_kcalloc (dev, reset_nb, +sizeof (struct reset_ctl ), +GFP_KERNEL ); +if (!priv->resets) +return -ENOMEM ; + +for (i =0 ; i < reset_nb; i++) { + err =reset_get_by_index (dev, i, &priv->resets[i]); +if (err <0 ) +break ; + + err =reset_deassert (&priv->resets[i]); +if (err) { +error ("failed to deassert reset %d\n" , i); +reset_free (&priv->resets[i]); + goto reset_err; + } + priv->reset_count++; +reset_free (&priv->resets[i]); + } + }else { +if (reset_nb != -ENOENT ) { +error ("failed to get reset phandle(%d)\n" , reset_nb); + goto clk_err; + } + } + err =ohci_register (dev, regs);if (err) - goto clk_err; + goto reset_err;return 0 ; +reset_err: + ret =reset_assert_all (priv->resets, priv->reset_count); +if (ret) +error ("failed to assert all resets\n" ); clk_err: ret =clk_disable_all (priv->clocks, priv->clock_count);if (ret)
@@ -80,6 +118,10 @@ static int ohci_usb_remove(struct udevice *dev)if (ret)return ret; + ret =reset_assert_all (priv->resets, priv->reset_count); +if (ret) +return ret; +return clk_disable_all (priv->clocks, priv->clock_count); }
Signature · 1 line
1.9.1
PATCH v8 10/10 usb: host: ohci-generic: add generic PHY support
In reply to patrice.chotard at st.comFrom: Patrice Chotard <[email protected]>
Extend ohci-generic driver with generic PHY framework
---
v8: _ rework error path by propagating the initial error code until the end of probe()
v7: _ none
v6: _ none
v5: _ none
v4: _ use generic_phy_valid() before generic_phy_exit() call
v3: _ extract in this patch the PHY support add-on from previous patch 5
drivers/usb/host/ohci-generic.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
drivers/usb/host/ohci-generic.c +30 -1
@@ -8,6 +8,7 @@#include <clk.h> #include <dm.h> #include <dm/ofnode.h> +#include <generic-phy.h> #include <reset.h> #include "ohci.h"
@@ -19,6 +20,7 @@ struct generic_ohci {ohci_t ohci;struct clk *clocks;struct reset_ctl *resets; +struct phy phy;int clock_count;int reset_count; };
@@ -91,12 +93,33 @@ static int ohci_usb_probe(struct udevice *dev) } } + err =generic_phy_get_by_index (dev,0 , &priv->phy); +if (err) { +if (err != -ENOENT ) { +error ("failed to get usb phy\n" ); + goto reset_err; + } + } + + err =generic_phy_init (&priv->phy); +if (err) { +error ("failed to init usb phy\n" ); + goto reset_err; + } + err =ohci_register (dev, regs);if (err) - goto reset_err; + goto phy_err;return 0 ; +phy_err: +if (generic_phy_valid (&priv->phy)) { + ret =generic_phy_exit (&priv->phy); +if (ret) +error ("failed to release phy\n" ); + } + reset_err: ret =reset_assert_all (priv->resets, priv->reset_count);if (ret)
@@ -118,6 +141,12 @@ static int ohci_usb_remove(struct udevice *dev)if (ret)return ret; +if (generic_phy_valid (&priv->phy)) { + ret =generic_phy_exit (&priv->phy); +if (ret) +return ret; + } + ret =reset_assert_all (priv->resets, priv->reset_count);if (ret)return ret;
Signature · 1 line
1.9.1
v8 cover usb: Extend ehci and ohci generic drivers
In reply to patrice.chotard at st.comDon't take care of this series, I need to resend a new one
Sorry
Quoted 78 lines · patrice.chotard at st.com
From: Patrice Chotard <[email protected]>
This series improves generic ehci and ohci drivers by addition of :
_ error path during probe (clocks, resets and phy release) _ .remove callback
_ add generic PHY framework for both generic ehci and ohci drivers _ add RESET and CLOCK framework for generic ohci driverTo implement these features, some new methods are needed in reset, clock and in dm/core framework:
_ addreset_request()andreset_assert_all()methods in RESET framework _ addclk_count()andclk_disable_all()methods in CLOCK framework _ addofnode_count_phandle_with_args()in dm/corev8: _ rework error path by propagating the initial error code until the end of
probe()
_ replacedevm_kmalloc()withdevm_kcalloc()_ fix cosmetics remarksv7: _ replace
clk_count()andreset_count()methods by
ofnode_count_phandle_with_args()in patches 3, 4 and 5v6: _ replace
clk_get_by_index()bydev_read_phandle_with_args()in
clk_count()in patch 4
_ add Reviewed-by Simon Glass for patch 2 and 5v5: _ rebase on top of dm/master requested by Simon Glass in order to use
livetree update
_ replacefdtdec_parse_phandle_with_args()bydev_read_phandle_with_args()in patch 2v4: _ add
clk_disable_all()andreset_assert_all()methods into CLOCK and
RESET framework as suggested by Simon Glass and Marek Vasut
_ addreset_count()andclk_count()methods which returns respectively the
number of resets and clocks declared into "resets" and "clocks" DT properties. This allows to allocate the right amount of memory to keep resets and clocks reference
_ update the memory allocation for deasserted resets and enabled
clocks reference list. Replace lists by arrays.v3: _ keep enabled clocks and deasserted resets reference in list in order to
disable clock or assert resets in error path or in .remove callback
_ add missing commit message
_ use structgeneric_ehci* instead of struct udevice * as parameter for
ehci_release_resets()andehci_release_clocks()
_ test return value ongeneric_phy_get_by_index()and
generic_phy_init()
_ split previous patch 5 in 3 independant patch for CLOCK, RESET and PHY supportv2: _ add needed
reset_request()in RESET framework
_ add error path in ehci/ohci-generic to disable clocks and to assert resets
_ add .remove callback with clocks, resets and phy release _ split the replacement ofprintf()byerror()in an independant patchPatrice Chotard (10):
reset: addreset_request()
reset: addreset_assert_all()
clk: addclk_disable_all()
dm: core: addofnode_count_phandle_with_args()
usb: host: ehci-generic: replaceprintf()byerror()
usb: host: ehci-generic: add error path and .remove callback
usb: host: ehci-generic: add generic PHY support
usb: host: ohci-generic: add CLOCK support
usb: host: ohci-generic: add RESET support
usb: host: ohci-generic: add generic PHY supportdrivers/clk/clk-uclass.c | 22 ++++++ drivers/core/of_access.c | 7 ++ drivers/core/ofnode.c | 12 ++++ drivers/reset/reset-uclass.c | 31 +++++++++ drivers/usb/host/ehci-generic.c | 149 +++++++++++++++++++++++++++++++++------- drivers/usb/host/ohci-generic.c | 130 ++++++++++++++++++++++++++++++++++- include/clk.h | 10 +++ include/dm/of_access.h | 18 +++++ include/dm/ofnode.h | 17 +++++ include/reset.h | 26 +++++++ 10 files changed, 397 insertions(+), 25 deletions(-)