All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] rockchip: clk: rv1108: remove duplicate reset init
Date: Sat, 09 Nov 2019 00:11:05 +0100	[thread overview]
Message-ID: <2585605.2uVP4U52sI@phil> (raw)
In-Reply-To: <3341784.rXeD3FAivv@diego>

Hi Kever,

Am Donnerstag, 7. November 2019, 10:13:02 CET schrieb Heiko Stübner:
> Am Donnerstag, 7. November 2019, 10:03:19 CET schrieb Kever Yang:
> > On 2019/10/24 上午1:45, Heiko Stuebner wrote:
> > > rockchip_reset_bind() already does the needed init for the reset
> > > registers, only referenced the wrong cru structure.
> > >
> > > So we can get rid of the open-coded reset init and just fix
> > > the correct cru reference.
> > >
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > >   drivers/clk/rockchip/clk_rv1108.c | 13 +------------
> > >   1 file changed, 1 insertion(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/clk/rockchip/clk_rv1108.c b/drivers/clk/rockchip/clk_rv1108.c
> > > index 3ebb007fab..5dc31e1eb0 100644
> > > --- a/drivers/clk/rockchip/clk_rv1108.c
> > > +++ b/drivers/clk/rockchip/clk_rv1108.c
> > > @@ -698,22 +698,11 @@ static int rv1108_clk_bind(struct udevice *dev)
> > >   	}
> > >   
> > >   #if CONFIG_IS_ENABLED(CONFIG_RESET_ROCKCHIP)
> > > -	ret = offsetof(struct rk3368_cru, softrst_con[0]);
> > > +	ret = offsetof(struct rv1108_cru, softrst_con[0]);
> > >   	ret = rockchip_reset_bind(dev, ret, 13);
> > >   	if (ret)
> > >   		debug("Warning: software reset driver bind faile\n");
> > >   #endif
> > > -	ret = device_bind_driver_to_node(dev, "rockchip_reset", "reset",
> > > -					 dev_ofnode(dev), &sf_child);
> > 
> > You can't just remove this blob of code, for there is a 'sysreset' 
> > driver and a 'reset' driver, they are
> > 
> > different, so you should fix this part to be available 'sysreset' driver 
> > so that the software can reset the SoC.
> 
> While there is a sysreset driver, it seems the rv1108 only did init
> the softrst code here and not the sysreset at all.
> 
> So rockchip_reset_bind really only does the same as the remove
> part of code. What I did see was that struct softreset_reg
> still is defined in the rockchip clock.h and after this patch no-one
> is using at all anymore. The rockchip_reset driver uses a different
> struct altogether it seems.
> 
> Adding an appropriate sysreset driver for rv1108
> should likely be a separate patch though.

I just sent a v2, dropping the unused softreset_reg struct as well.

When reviewing please take into account, the rv1108 had the following
situation before:

-registration of sysreset
- registration of old softreset - broken because that driver code is gone
- registration of new softreset - broken because of the CONFIG_* from patch2
			and also because it was using the wrong struct (rk3368 instead of rv1108)

now after the patch (in v2), we have the expected
- registration of sysreset
- registration of new softreset with correct cru struct

Heiko

      reply	other threads:[~2019-11-08 23:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-23 17:45 [U-Boot] [PATCH 1/2] rockchip: clk: rv1108: remove duplicate reset init Heiko Stuebner
2019-10-23 17:45 ` [U-Boot] [PATCH 2/2] rockchip: clk: fix wrong CONFIG_IS_ENABLED handling Heiko Stuebner
2019-11-07  9:04   ` Kever Yang
2019-11-07  9:03 ` [U-Boot] [PATCH 1/2] rockchip: clk: rv1108: remove duplicate reset init Kever Yang
2019-11-07  9:13   ` Heiko Stübner
2019-11-08 23:11     ` Heiko Stuebner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2585605.2uVP4U52sI@phil \
    --to=heiko@sntech.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.