Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
On 05/11/2019 17:42, Simon Goldschmidt wrote: Am 05.11.2019 um 17:33 schrieb Simon Glass: Hi Jean-Jacques, On Mon, 4 Nov 2019 at 08:41, Jean-Jacques Hiblot wrote: On 30/10/2019 02:48, Simon Glass wrote: On Mon, 30 Sep 2019 at 10:15, Jean-Jacques Hiblot wrote: Add managed functions to get a reset_ctl from the device-tree, based on a name or an index. Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) from the device-tree. When the device is unbound, the reset controllers are automatically released and the data structure is freed. Signed-off-by: Jean-Jacques Hiblot --- drivers/reset/reset-uclass.c | 116 +- include/reset.h | 135 ++- 2 files changed, 247 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass I really don't like these ERR_PTR returns. I suppose they make the code easier to port, and we can be sure that pointers will not be in the last 4KB of address space? It seems rather unlikely because the returned pointer points to actual RAM allocated from the heap. On most platforms I've worked with, the top of the address space is not dedicated to memory. Most != all: on socfpga, the internal SRAM is at the end of the address spcae. In SPL, this means the last 4K cannot be used. However, that shouldn't keep us from porting ERR_PTR returns from Linux code. Yes that's my comfort. If ever the need to fix this arises it could done by tweaking the macros to use another unused address space. Not easily without doing something platform-specific, as someone else is currently pushing. That "someone else" would be me. Sadly, I did not get any response: https://patchwork.ozlabs.org/project/uboot/list/?series=137880 Alternatively we could use BIT(0) to flag an error since allocations are always aligned on 4 or more (sizeof(ulong) or 2*sizeof(size_t)) Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
Am 05.11.2019 um 17:33 schrieb Simon Glass: Hi Jean-Jacques, On Mon, 4 Nov 2019 at 08:41, Jean-Jacques Hiblot wrote: On 30/10/2019 02:48, Simon Glass wrote: On Mon, 30 Sep 2019 at 10:15, Jean-Jacques Hiblot wrote: Add managed functions to get a reset_ctl from the device-tree, based on a name or an index. Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) from the device-tree. When the device is unbound, the reset controllers are automatically released and the data structure is freed. Signed-off-by: Jean-Jacques Hiblot --- drivers/reset/reset-uclass.c | 116 +- include/reset.h | 135 ++- 2 files changed, 247 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass I really don't like these ERR_PTR returns. I suppose they make the code easier to port, and we can be sure that pointers will not be in the last 4KB of address space? It seems rather unlikely because the returned pointer points to actual RAM allocated from the heap. On most platforms I've worked with, the top of the address space is not dedicated to memory. Most != all: on socfpga, the internal SRAM is at the end of the address spcae. In SPL, this means the last 4K cannot be used. However, that shouldn't keep us from porting ERR_PTR returns from Linux code. Yes that's my comfort. If ever the need to fix this arises it could done by tweaking the macros to use another unused address space. Not easily without doing something platform-specific, as someone else is currently pushing. That "someone else" would be me. Sadly, I did not get any response: https://patchwork.ozlabs.org/project/uboot/list/?series=137880 Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
Hi Jean-Jacques, On Mon, 4 Nov 2019 at 08:41, Jean-Jacques Hiblot wrote: > > > On 30/10/2019 02:48, Simon Glass wrote: > > On Mon, 30 Sep 2019 at 10:15, Jean-Jacques Hiblot wrote: > >> Add managed functions to get a reset_ctl from the device-tree, based on a > >> name or an index. > >> Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) > >> from the device-tree. > >> > >> When the device is unbound, the reset controllers are automatically > >> released and the data structure is freed. > >> > >> Signed-off-by: Jean-Jacques Hiblot > >> --- > >> > >> drivers/reset/reset-uclass.c | 116 +- > >> include/reset.h | 135 ++- > >> 2 files changed, 247 insertions(+), 4 deletions(-) > > Reviewed-by: Simon Glass > > > > I really don't like these ERR_PTR returns. I suppose they make the > > code easier to port, and we can be sure that pointers will not be in > > the last 4KB of address space? > > It seems rather unlikely because the returned pointer points to actual > RAM allocated from the heap. On most platforms I've worked with, the top > of the address space is not dedicated to memory. Yes that's my comfort. > If ever the need to fix > this arises it could done by tweaking the macros to use another unused > address space. Not easily without doing something platform-specific, as someone else is currently pushing. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
On 30/10/2019 02:48, Simon Glass wrote: On Mon, 30 Sep 2019 at 10:15, Jean-Jacques Hiblot wrote: Add managed functions to get a reset_ctl from the device-tree, based on a name or an index. Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) from the device-tree. When the device is unbound, the reset controllers are automatically released and the data structure is freed. Signed-off-by: Jean-Jacques Hiblot --- drivers/reset/reset-uclass.c | 116 +- include/reset.h | 135 ++- 2 files changed, 247 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass I really don't like these ERR_PTR returns. I suppose they make the code easier to port, and we can be sure that pointers will not be in the last 4KB of address space? It seems rather unlikely because the returned pointer points to actual RAM allocated from the heap. On most platforms I've worked with, the top of the address space is not dedicated to memory. If ever the need to fix this arises it could done by tweaking the macros to use another unused address space. JJ Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
On Mon, 30 Sep 2019 at 10:15, Jean-Jacques Hiblot wrote: > > Add managed functions to get a reset_ctl from the device-tree, based on a > name or an index. > Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) > from the device-tree. > > When the device is unbound, the reset controllers are automatically > released and the data structure is freed. > > Signed-off-by: Jean-Jacques Hiblot > --- > > drivers/reset/reset-uclass.c | 116 +- > include/reset.h | 135 ++- > 2 files changed, 247 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass I really don't like these ERR_PTR returns. I suppose they make the code easier to port, and we can be sure that pointers will not be in the last 4KB of address space? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT
Add managed functions to get a reset_ctl from the device-tree, based on a name or an index. Also add a managed functions to get a reset_ctl_bulk (array of reset_ctl) from the device-tree. When the device is unbound, the reset controllers are automatically released and the data structure is freed. Signed-off-by: Jean-Jacques Hiblot --- drivers/reset/reset-uclass.c | 116 +- include/reset.h | 135 ++- 2 files changed, 247 insertions(+), 4 deletions(-) diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c index 1cfcc8b367..ae8395a5e9 100644 --- a/drivers/reset/reset-uclass.c +++ b/drivers/reset/reset-uclass.c @@ -8,6 +8,7 @@ #include #include #include +#include struct reset_ops nop_reset_ops = { }; @@ -101,13 +102,14 @@ int reset_get_by_index_nodev(ofnode node, int index, index > 0, reset_ctl); } -int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk) +static int __reset_get_bulk(struct udevice *dev, ofnode node, + struct reset_ctl_bulk *bulk) { int i, ret, err, count; bulk->count = 0; - count = dev_count_phandle_with_args(dev, "resets", "#reset-cells"); + count = ofnode_count_phandle_with_args(node, "resets", "#reset-cells"); if (count < 1) return count; @@ -117,7 +119,7 @@ int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk) return -ENOMEM; for (i = 0; i < count; i++) { - ret = reset_get_by_index(dev, i, &bulk->resets[i]); + ret = reset_get_by_index_nodev(node, i, &bulk->resets[i]); if (ret < 0) goto bulk_get_err; @@ -135,6 +137,11 @@ bulk_get_err: return ret; } +int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk) +{ + return __reset_get_bulk(dev, dev_ofnode(dev), bulk); +} + int reset_get_by_name(struct udevice *dev, const char *name, struct reset_ctl *reset_ctl) { @@ -247,6 +254,109 @@ int reset_release_all(struct reset_ctl *reset_ctl, int count) return 0; } +static void devm_reset_release(struct udevice *dev, void *res) +{ + reset_free(res); +} + +struct reset_ctl *devm_reset_control_get_by_index(struct udevice *dev, + int index) +{ + int rc; + struct reset_ctl *reset_ctl; + + reset_ctl = devres_alloc(devm_reset_release, sizeof(struct reset_ctl), +__GFP_ZERO); + if (unlikely(!reset_ctl)) + return ERR_PTR(-ENOMEM); + + rc = reset_get_by_index(dev, index, reset_ctl); + if (rc) + return ERR_PTR(rc); + + devres_add(dev, reset_ctl); + return reset_ctl; +} + +struct reset_ctl *devm_reset_control_get(struct udevice *dev, const char *id) +{ + int rc; + struct reset_ctl *reset_ctl; + + reset_ctl = devres_alloc(devm_reset_release, sizeof(struct reset_ctl), +__GFP_ZERO); + if (unlikely(!reset_ctl)) + return ERR_PTR(-ENOMEM); + + rc = reset_get_by_name(dev, id, reset_ctl); + if (rc) + return ERR_PTR(rc); + + devres_add(dev, reset_ctl); + return reset_ctl; +} + +struct reset_ctl *devm_reset_control_get_optional(struct udevice *dev, + const char *id) +{ + struct reset_ctl *r = devm_reset_control_get(dev, id); + + if (IS_ERR(r)) + return NULL; + + return r; +} + +static void devm_reset_bulk_release(struct udevice *dev, void *res) +{ + struct reset_ctl_bulk *bulk = res; + + reset_release_all(bulk->resets, bulk->count); +} + +struct reset_ctl_bulk *devm_reset_bulk_get_by_node(struct udevice *dev, + ofnode node) +{ + int rc; + struct reset_ctl_bulk *bulk; + + bulk = devres_alloc(devm_reset_bulk_release, + sizeof(struct reset_ctl_bulk), + __GFP_ZERO); + if (unlikely(!bulk)) + return ERR_PTR(-ENOMEM); + + rc = __reset_get_bulk(dev, node, bulk); + if (rc) + return ERR_PTR(rc); + + devres_add(dev, bulk); + return bulk; +} + +struct reset_ctl_bulk *devm_reset_bulk_get_optional_by_node(struct udevice *dev, + ofnode node) +{ + struct reset_ctl_bulk *bulk; + + bulk = devm_reset_bulk_get_by_node(dev, node); + + if (IS_ERR(bulk)) + return NULL; + + return bulk; +} + +struct reset_ctl_bulk *devm_reset_bulk_get(struct udevice *dev) +{ + return devm_reset_bulk_get_by_node(dev, dev_ofnode(dev)); +} + +struct reset_ctl_bulk *devm_reset_bulk_get_optional(struct udevice *dev) +{