[U-Boot] [PATCH] sysreset: syscon: update regmap access to syscon

[U-Boot] [PATCH] sysreset: syscon: update regmap access to syscon

[Patrick Delaunay]

Use new API syscon_node_to_regmap in sysreset_syscon driver
for compatible "syscon-reboot"; that's avoid the need of explicit
syscon binding for "regmap" handle.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 drivers/sysreset/sysreset_syscon.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/sysreset/sysreset_syscon.c b/drivers/sysreset/sysreset_syscon.c
index f19e80e..3450640 100644
--- a/drivers/sysreset/sysreset_syscon.c
+++ b/drivers/sysreset/sysreset_syscon.c
@@ -35,18 +35,20 @@ static struct sysreset_ops syscon_reboot_ops = {
 
 int syscon_reboot_probe(struct udevice *dev)
 {
-	struct udevice *syscon;
 	struct syscon_reboot_priv *priv = dev_get_priv(dev);
 	int err;
+	u32 phandle;
+	ofnode node;
 
-	err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev,
-					   "regmap", &syscon);
-	if (err) {
-		pr_err("unable to find syscon device\n");
+	err = ofnode_read_u32(dev_ofnode(dev), "regmap", &phandle);
+	if (err)
 		return err;
-	}
 
-	priv->regmap = syscon_get_regmap(syscon);
+	node = ofnode_get_by_phandle(phandle);
+	if (!ofnode_valid(node))
+		return -EINVAL;
+
+	priv->regmap = syscon_node_to_regmap(node);
 	if (!priv->regmap) {
 		pr_err("unable to find regmap\n");
 		return -ENODEV;
-- 
2.7.4

[U-Boot] [PATCH] sysreset: syscon: update regmap access to syscon

[Simon Glass]

Hi Patrick,

On 9 July 2018 at 06:59, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> Use new API syscon_node_to_regmap in sysreset_syscon driver
> for compatible "syscon-reboot"; that's avoid the need of explicit
> syscon binding for "regmap" handle.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/sysreset/sysreset_syscon.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/sysreset/sysreset_syscon.c b/drivers/sysreset/sysreset_syscon.c
> index f19e80e..3450640 100644
> --- a/drivers/sysreset/sysreset_syscon.c
> +++ b/drivers/sysreset/sysreset_syscon.c
> @@ -35,18 +35,20 @@ static struct sysreset_ops syscon_reboot_ops = {
>
>  int syscon_reboot_probe(struct udevice *dev)
>  {
> -       struct udevice *syscon;
>         struct syscon_reboot_priv *priv = dev_get_priv(dev);
>         int err;
> +       u32 phandle;
> +       ofnode node;
>
> -       err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev,
> -                                          "regmap", &syscon);
> -       if (err) {
> -               pr_err("unable to find syscon device\n");
> +       err = ofnode_read_u32(dev_ofnode(dev), "regmap", &phandle);
> +       if (err)
>                 return err;
> -       }
>
> -       priv->regmap = syscon_get_regmap(syscon);
> +       node = ofnode_get_by_phandle(phandle);
> +       if (!ofnode_valid(node))
> +               return -EINVAL;
> +
> +       priv->regmap = syscon_node_to_regmap(node);
>         if (!priv->regmap) {
>                 pr_err("unable to find regmap\n");
>                 return -ENODEV;

Aren't you just re-implementing uclass_get_device_by_phandle() here?

Regards,
Simon

[U-Boot] [PATCH] sysreset: syscon: update regmap access to syscon

[Patrick DELAUNAY]

Hi Simon

> -----Original Message-----
> From: sjg at google.com <sjg@google.com> On Behalf Of Simon Glass
> Sent: mardi 10 juillet 2018 22:50
> 
> Hi Patrick,
> 
> On 9 July 2018 at 06:59, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> > Use new API syscon_node_to_regmap in sysreset_syscon driver for
> > compatible "syscon-reboot"; that's avoid the need of explicit syscon
> > binding for "regmap" handle.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  drivers/sysreset/sysreset_syscon.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/sysreset/sysreset_syscon.c
> > b/drivers/sysreset/sysreset_syscon.c
> > index f19e80e..3450640 100644
> > --- a/drivers/sysreset/sysreset_syscon.c
> > +++ b/drivers/sysreset/sysreset_syscon.c
> > @@ -35,18 +35,20 @@ static struct sysreset_ops syscon_reboot_ops = {
> >
> >  int syscon_reboot_probe(struct udevice *dev)  {
> > -       struct udevice *syscon;
> >         struct syscon_reboot_priv *priv = dev_get_priv(dev);
> >         int err;
> > +       u32 phandle;
> > +       ofnode node;
> >
> > -       err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev,
> > -                                          "regmap", &syscon);
> > -       if (err) {
> > -               pr_err("unable to find syscon device\n");
> > +       err = ofnode_read_u32(dev_ofnode(dev), "regmap", &phandle);
> > +       if (err)
> >                 return err;
> > -       }
> >
> > -       priv->regmap = syscon_get_regmap(syscon);
> > +       node = ofnode_get_by_phandle(phandle);
> > +       if (!ofnode_valid(node))
> > +               return -EINVAL;
> > +
> > +       priv->regmap = syscon_node_to_regmap(node);
> >         if (!priv->regmap) {
> >                 pr_err("unable to find regmap\n");
> >                 return -ENODEV;
> 
> Aren't you just re-implementing uclass_get_device_by_phandle() here?

Not exactly, because if I use the the function uclass_get_device_by_phandle, the device need to be bounded to syscon UCLASS.

And it is not needed with my proposal as I use the new function syscon_node_to_regmap which not require explicite syscon binding.

I have the problem in stm32mp1 platform with the DT : (./arch/arm/dts/stm32mp157c.dtsi)

		rcc: rcc at 50000000 {
			compatible = "st,stm32mp1-rcc", "syscon";
			#clock-cells = <1>;
			#reset-cells = <1>;
			reg = <0x50000000 0x1000>;
			st,pwr = <&pwr>;
			interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
				     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
		};

		rcc_reboot: rcc-reboot at 50000000 {
			compatible = "syscon-reboot";
			regmap = <&rcc>;
			offset = <0x404>;
			mask = <0x1>;
		};

1/ rcc is binded to drivers/misc/stm32_rcc.c with compatible "st,stm32mp1-rcc"
    =>3 roles
    + Clock provider => bound with stm32mp1_clk driver
    + Reset provide => bound with "stm32_rcc_reset"
   + sysreset provider with syscon.... not bound by default

2/ rcc-reboot is binded to "sysreset_syscon" driver 
	=> regmap field = handle to rcc

Without modification, the syscon-reboot driver can't have access to regmap of RCC because it is not bound to generic syscon driver.

I have 2 solutions to solve this:

1/ add a 3rd binding to syscon in RCC misc driver
	if (rcc_clk->soc == STM32MP1) {
		ret = device_bind_driver_to_node(dev, "syscon",
						 "syscon",
						 dev_ofnode(dev), &child);
		if (ret)
			return ret;
	}

2/ change sysreset to access directly to syscon regmap without explicate binding.

I choose the 2nd solution, I think it is more generic.

NB:  I can change the patch to introduce a new syscon function to access to syscon form phandle if it is preferable
        struct regmap * syscon_handle_to_regmap(struct udevice *parent,  const char *name)

        or I can come back to the first solution (add the binding to syscon)

> Regards,
> Simon

Regards
Patrick

[U-Boot] sysreset: syscon: update regmap access to syscon

[Tom Rini]

On Mon, Jul 09, 2018 at 02:59:29PM +0200, Patrick Delaunay wrote:

> Use new API syscon_node_to_regmap in sysreset_syscon driver
> for compatible "syscon-reboot"; that's avoid the need of explicit
> syscon binding for "regmap" handle.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/ddaa44c4/attachment.sig>