linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices
Date: Thu, 25 Sep 2014 14:42 +0200	[thread overview]
Message-ID: <5439567.VeRyeUcp1j@wuerfel> (raw)
In-Reply-To: <000001cfd719$51e7e560$f5b7b020$@samsung.com>

On Tuesday 23 September 2014 15:59:43 Pankaj Dubey wrote:
> > > -static int syscon_match_node(struct device *dev, void *data)
> > > +static struct syscon *of_syscon_register(struct device_node *np)
> > >  {
> > > -   struct device_node *dn = data;
> > > +   struct platform_device *pdev = NULL;
> > > +   struct syscon *syscon;
> > > +   struct regmap *regmap;
> > > +   void __iomem *base;
> > > +   int ret;
> > > +
> > > +   if (!of_device_is_compatible(np, "syscon"))
> > > +           return ERR_PTR(-EINVAL);
> > > +
> > > +   syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> > > +   if (!syscon)
> > > +           return ERR_PTR(-ENOMEM);
> > > +
> > > +   base = of_iomap(np, 0);
> > > +   if (!base) {
> > > +           ret = -ENOMEM;
> > > +           goto err_map;
> > > +   }
> > > +
> > > +   /* if device is already populated and available then use it */
> > > +   pdev = of_find_device_by_node(np);
> > > +   if (!pdev) {
> > > +           /* for early users create dummy syscon device and use it */
> > > +           pdev = platform_device_alloc("dummy-syscon", -1);
> > 
> > Can we create specific devices for each syscon device?
> > That looks more reasonable to me.
> > Then we can dump registers in regmap debugfs more clearly, just like other
> devices.
> > And i think it's better to follow device tree way to generate devices from
> node.
> > If DT core find a device is already created, it can ignore that device to
> avoid creating
> > duplicated device during of_platform_populate.
> > 
> 
> If you are referring to use "of_platform_device_create", then I am afraid
> that it can't be used
> in very early stage. For example, current patch I tested by calling
> syscon_lookup_by_phandle from
> "init_irq" hook of machine_desc, and it worked well, but If I use
> "of_platform_device_create"
> instead of dummy device, it fails to create pdev and this I verified on
> Exynos platform. The reason
> I selected "init_irq" is because this comes just before smp init and where
> once we tried to get regmap handle,
> but could not do so. 

I don't remember noticing the of_find_device_by_node or platform_device_alloc
in earlier versions of this patch, but that could just be me failing to
read it right.

I think this should get removed: it would be much better to ensure that
the regmap code can work without a device pointer, which I thought it
did. We should probably also skip the creation of the debugfs directory
in that case.

> Also if it was just a matter of creating platform_device at early stage then
> early initialization of
> syscon could have been solved till now.
> 
> So I would suggest if we really want this patch to cover early
> initialization of syscon
> (which is not it's main purpose) current patch is sufficient. 
> 
> @Arnd, since I have addressed crash issue as reported by Dong Aisheng, I
> would like to take your ack,
> but since there are some more code changes other than what you suggested I
> request you to check this
> and if you are ok, then I would like to your ack so that I can request to
> maintainer for taking this patch.

I think we first need to resolve the question of the platform device.

	Arnd

  reply	other threads:[~2014-09-25 12:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-22  4:40 [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices Pankaj Dubey
2014-09-22  9:19 ` Dong Aisheng
2014-09-23 10:29   ` Pankaj Dubey
2014-09-25 12:42     ` Arnd Bergmann [this message]
2014-09-26  5:28       ` Pankaj Dubey
2014-09-26  7:14         ` Arnd Bergmann
2014-09-22  9:55 ` Li.Xiubo at freescale.com
2014-09-23 18:12 ` Joachim Eastwood
2014-09-24 18:35   ` Heiko Stübner
2014-09-25 12:21     ` Heiko Stübner
2014-09-26  4:56       ` Pankaj Dubey
2014-09-26  5:34         ` Joachim Eastwood
2014-09-26  7:16           ` Arnd Bergmann
2014-09-26  7:48             ` Joachim Eastwood
2014-09-26  9:14               ` Arnd Bergmann

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=5439567.VeRyeUcp1j@wuerfel \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 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).