Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1647095774.git.tonyhuang.sunplus@gmail.com>
 <07f507a84a9e39d3cd8393f41d1292c250e07642.1647095774.git.tonyhuang.sunplus@gmail.com>
 <00362767-080a-aa7f-672f-22b83ab35e61@kernel.org> <42f5f710077b40d7b6fde45789f46732@sphcmbx02.sunplus.com.tw>
 <e4ed4b50-c16f-5c46-7d64-f5b2f36f8f44@kernel.org> <58fd70a3fa3d44229edd849cab49eadf@sphcmbx02.sunplus.com.tw>
 <f3c7af9b-85a0-1148-6096-30345f1a1478@kernel.org> <0d615380aa85444c81e9b93f4a216232@sphcmbx02.sunplus.com.tw>
In-Reply-To: <0d615380aa85444c81e9b93f4a216232@sphcmbx02.sunplus.com.tw>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Tue, 15 Mar 2022 10:22:45 +0100
Message-ID: <CAK8P3a2K+TTP3a_jNsPi6Ea+8THrM31ecO_=bNEFqPP+XZCd9w@mail.gmail.com>
Subject: Re: [PATCH v11 2/2] misc: Add iop driver for Sunplus SP7021
To:     =?UTF-8?B?VG9ueSBIdWFuZyDpu4Pmh7fljpo=?= <tony.huang@sunplus.com>
Cc:     Krzysztof Kozlowski <krzk@kernel.org>,
        Tony Huang <tonyhuang.sunplus@gmail.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "derek.kiernan@xilinx.com" <derek.kiernan@xilinx.com>,
        "dragan.cvetic@xilinx.com" <dragan.cvetic@xilinx.com>,
        "arnd@arndb.de" <arnd@arndb.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        =?UTF-8?B?V2VsbHMgTHUg5ZGC6Iqz6aiw?= <wells.lu@sunplus.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Mar 15, 2022 at 9:24 AM Tony Huang 黃懷厚 <tony.huang@sunplus.com> wrote:
> > >>>> On 12/03/2022 17:16, Tony Huang wrote:

(note: please use normal quoting style, only quote the parts of the message
 that you actually reply to, reave the attributoons in place, etc)

> > > Can I put sp_iop_poweroff() here for now?
> > > When power off device driver is added in /driver/power/reset is complete,
> > we will move to it.
> >
> > Not really, because misc drivers is not a place for power off drivers.
> > The driver here looks now like responsible only for system power management
> > of a SoC, so most likely drivers/soc. However it has even less sense to add
> > some feature here and immediately move it somewhere more appropriate
> > (instead just add it to the appropriate place).
> >
> > Your moving of parts of it to drivers/power/reset is now confusing. What will
> > be left here? Please send entire set not just pieces.
> >
>
> I may not fully understand your requirements.
> You: What is your plan for this driver?
> < ----- reset driver? Misc driver?
> You: However keeping all in the same place would be ok, if you do not plan to add any more features.
> This would mean the driver will handle *only* reset and FW loading.
> < ------         Because system does not have a power off device driver.
>         sp_iop_poweroff() can be placed in iop driver?


It really depends on where you want this driver to head. If you don't
have any plans to add features beyond poweroff, having the whole
thing in drivers/power/reset/ is the easiest way. If you do have plans
to add other features, then explain exactly what those features are,
which subsystems they go into, and why you haven't already
implemented them in the previous 11 versions.

The common bits of the driver can go into drivers/soc or drivers/mfd,
partly depending on what abstraction you use between this
driver and its clients, so it's important to figure out the correct
abstraction layer first, and then decide where it should go.

One common way to do it is to use the multi-function-device
abstraction to automatically create child devices from the
hardware device node. Another option is to have high-level
functions exported from a soc driver. Or you could just have
a 'regmap' exposed from a 'syscon' driver to provide
individual registers to devices. Which one of those is best
for your system mostly depends on how you end up using
it, and we have no idea until you have more than one leaf
driver on it. The poweroff driver is not a great example either,
because it does not have an interesting interface and just
comes down to a single function call.

      Arnd