Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 17 May 2022 15:01:36 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Stephen Boyd <sboyd@kernel.org>
Cc:     Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        Jingoo Han <jingoohan1@gmail.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Serge Semin <Sergey.Semin@baikalelectronics.ru>,
        Serge Semin <fancer.lancer@gmail.com>,
        Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
        Rob Herring <robh@kernel.org>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        linux-clk@vger.kernel.org, linux-pci@vger.kernel.org,
        linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/4] clk: baikal-t1: Move reset-controls code into a
 dedicated module
Message-ID: <20220517120136.mg6ihyzjb6rme6lc@mobilestation>
References: <20220503205722.24755-1-Sergey.Semin@baikalelectronics.ru>
 <20220503205722.24755-4-Sergey.Semin@baikalelectronics.ru>
 <20220517073729.2FAE2C385B8@smtp.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220517073729.2FAE2C385B8@smtp.kernel.org>
Precedence: bulk

On Tue, May 17, 2022 at 12:37:26AM -0700, Stephen Boyd wrote:
> Quoting Serge Semin (2022-05-03 13:57:21)
> > Before adding the directly controlled resets support it's reasonable to
> > move the existing resets control functionality into a dedicated object for
> > the sake of the CCU dividers clock driver simplification. After the new
> > functionality is added clk-ccu-div.c would have got to a mixture of the
> > weakly dependent clocks and resets methods. Splitting the methods up into
> > the two objects will make code easier to read especially seeing it isn't
> > that hard to do.
> > 
> > As before the CCU reset module will support the trigger-like CCU resets
> > only, which are responsible for the AXI-bus, APB-bus and SATA-ref blocks
> > reset. The assert/de-assert-capable reset controls support will be added
> > in the next commit.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  drivers/clk/baikal-t1/Kconfig       |  12 +-
> >  drivers/clk/baikal-t1/Makefile      |   1 +
> >  drivers/clk/baikal-t1/ccu-rst.c     | 258 ++++++++++++++++++++++++++++
> >  drivers/clk/baikal-t1/ccu-rst.h     |  60 +++++++
> >  drivers/clk/baikal-t1/clk-ccu-div.c |  94 ++--------
> 

> Perhaps this should be done via the auxiliary bus by having the clk
> driver register the reset driver and have some private API to pass any
> data to the reset driver? Then the whole file could be in
> drivers/reset/, reviewed and maintained by the reset maintainer.

I'd rather stick to the current design in the same way as the most of
the OF-based platform clock drivers. Completely splitting the reset
and clock parts isn't that easy is it seems like (though the main part
has been done in the framework of this patchset). AXI/APB-interface
reset controls are tightly coupled with the AXI/APB-clock domains at
least by having the same CSRs. Due to that the corresponding AXI-clock
domains reset functionality will need to be left in the
drivers/clk/baikal-t1/clk-ccu* module. It makes the movement isn't
that justified and will result in needless code complications and the
code coherency worsening.

-Sergey