Re: [U-Boot] [PATCH v1 2/3] drivers: reset: Add a managed API to get reset controllers from the DT

2019-11-05 Thread Jean-Jacques Hiblot


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

2019-11-05 Thread Simon Goldschmidt

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

2019-11-05 Thread 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.

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

2019-11-04 Thread Jean-Jacques Hiblot


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

2019-10-29 Thread Simon Glass
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

2019-09-30 Thread Jean-Jacques Hiblot
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)
+{