LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: "fan.chen" <fan.chen@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>, YH Chen <yh.chen@mediatek.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Henry Chen <henryc.chen@mediatek.com>,
	Flora Fu <flora.fu@mediatek.com>,
	linux-mediatek@lists.infradead.org,
	=Sascha Hauer <kernel@pengutronix.de>,
	Olof Johansson <olof@lixom.net>,
	Yingjoe Chen <Yingjoe.Chen@mediatek.com>,
	Eddie Huang <eddie.huang@mediatek.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] soc: mediatek: Add PMIC wrapper for MT8135 and MT8173 SoCs
Date: Tue, 10 Mar 2015 13:07:43 +0100	[thread overview]
Message-ID: <20150310120743.GG24885@pengutronix.de> (raw)
In-Reply-To: <1425986519.28516.59.camel@mtksdaap41>

On Tue, Mar 10, 2015 at 07:21:59PM +0800, fan.chen wrote:
> On Mon, 2015-03-09 at 11:38 +0100, Sascha Hauer wrote:
> > > > +
> > > > +       switch (ck_mhz) {
> > > > +       case 18:
> > > > +               if (pwrap_is_mt8135(wrp))
> > > > +                       pwrap_writel(wrp, 0xc, PWRAP_CSHEXT);
> > > 
> > > If the pmic-wrapper is unique to every SoC, we should think of
> > > providing a different way to distinguish between the implementations.
> > > Otherwise we will bloat up the code with SoC specific conditionals.
> > 
> > I am happy to replace this with pwrap_is_v2() or even with additional
> > function hooks in struct pmic_wrapper_type once it's clear how other
> > pmic wrapper are different. Without this knowledge it's hard to predict
> > what the right weapon is to abstract between SoC differences.
> > 
> > Sascha
> > 
> Per function similarity, the pmic-wrapper of a new soc usually comes
> from an existing soc and have some modifications on it. (it does not
> have exactly the same version shared between SoCs so far)

So you don't have some internal versioning scheme, right? In that case the
SoC name is the best we can use as a version.

> In this case, MT8173's pmic wrapper is derived from MT8135 and they
> share much common implementation.
> For other SOCs like 6582/6592, they would have different init flows, but
> the pwrap-read/write part has very similar protocol. 
> The driver is originally intended for MT8135/MT8173 since they share quite common init flow.

Originally there were two drivers for both SoCs, it was me who merged
them.

> For other SOCs, they won't fit in current MT8135/MT173 init flow.
> Should we move pmic-wrapper init part to a chip-specific file and leave
> only common part (pmic read/write protocol) for all SoC here? ex:

Putting these into separate files doesn't change much. If this becomes
necessary because otherwise the driver grows too big it can always be
done. For now I have created an abstraction which fits for two SoC
derivates. If more derivates come then the abstraction has to grow with
the situation. Yes, we don't want to end up with stuff like
if (is_mt8173() || is_mt8135() || (is_mt6xxx() && !is_mt6533())).
Everytime another is_mtxy() is added we have to look if it's still the
right thing to be do or if function pointers in SoC specific data
structs are better, or maybe a completely other approach.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  parent reply	other threads:[~2015-03-10 12:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-22 12:02 [PATCH] Mediatek PMIC wrapper support Sascha Hauer
2015-02-22 12:02 ` [PATCH 1/2] soc: mediatek: Add PMIC wrapper for MT8135 and MT8173 SoCs Sascha Hauer
2015-03-09  8:53   ` Matthias Brugger
2015-03-09 10:38     ` Sascha Hauer
     [not found]       ` <1425986519.28516.59.camel@mtksdaap41>
2015-03-10 12:07         ` Sascha Hauer [this message]
2015-02-22 12:02 ` [PATCH 2/2] dt-bindings: ARM: Mediatek: document binding for the PMIC wrapper Sascha Hauer
2015-03-09  5:37 ` [PATCH] Mediatek PMIC wrapper support Sascha Hauer

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=20150310120743.GG24885@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=Yingjoe.Chen@mediatek.com \
    --cc=arnd@arndb.de \
    --cc=eddie.huang@mediatek.com \
    --cc=fan.chen@mediatek.com \
    --cc=flora.fu@mediatek.com \
    --cc=henryc.chen@mediatek.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=olof@lixom.net \
    --cc=yh.chen@mediatek.com \
    --subject='Re: [PATCH 1/2] soc: mediatek: Add PMIC wrapper for MT8135 and MT8173 SoCs' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).