All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Tue, 5 Nov 2013 19:48:11 +0100	[thread overview]
Message-ID: <201311051948.11992.arnd@arndb.de> (raw)
In-Reply-To: <20131105180314.GB14706@obsidianresearch.com>

On Tuesday 05 November 2013, Jason Gunthorpe wrote:
> I've been brewing a patch set to fix TPM, but it is a large effort. I
> would think other legacy subsystems that are not fully class device
> enabled will have similar challenges?

I think a common pattern is to have the subsystem data be referenced from
the driver data using a pointer, and contain a back reference to the struct
device, which should not be a problem.

> How about an alternate entry point:
> 
>  net = alloc_etherdev(sizeof(foo_priv));
>  priv = netdev_priv(net);
>  devm_probe_priv(priv, probes);
> 
> Don't touch the drvdata for that flow.

Yes, that would work, but it would prevent another idea I had that I haven't
mentioned here: If we add a pointer to the 'struct devm_probe' array to
'struct device_driver', those can actually be called by the driver core
code before entering the probe() callback, or potentially replace the
probe() function entirely for simple drivers (provided we add a few more
bits that we haven't talked about here.

I was originally thinking of something like

int devm_probe_alloc_netdev(struct device *dev, struct devm_probe *probe)
{
	struct net_device *netdev, **netdevp;

	/* open-coded devm_alloc_ethernet */
	netdevp = devres_alloc(devm_free_netdev, sizeof (*netdevp), GFP_KERNEL);
	if (!netdevp)
		return -ENOMEM;

	netdev = alloc_etherdev(probe->size);
	if (!netdev) {
		devres_free(netdevp);
		return -ENOMEM;
	}

	*netdevp = netdev;
	devres_add(dev, netdevp);

	dev_set_drvdata(dev, netdev_priv(netdev));

        netdevp = dev_get_drvdata(dev) + probe->offset;
        *netdevp = netdev;
}
#define DEVM_ALLOC_NETDEV(_struct, _member) {				\
	.initfunc = devm_probe_alloc_netdev,				\
	.size = sizeof(struct _struct),					\
	.offset = offsetof_t(struct _struct, _member, struct netdev *), \
}

which would fit in nicely with the rest of the design, but now I'm no longer
sure if that would actually work with the lifetime rules of the netdev, which
would pin the refcount on the 'struct device', which in turn could prevent
the devres cleanup to be executed.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	Jon Loeliger <jdl@jdl.com>,
	khilman@linaro.org, Stephen Warren <swarren@nvidia.com>,
	Benoit Cousson <bcousson@baylibre.com>,
	pawel.moll@arm.com, Stephen Warren <swarren@wwwdotorg.org>,
	Tomasz Figa <t.figa@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	rob.herring@calxeda.com, Grant Likely <grant.likely@secretlab.ca>,
	fparent@baylibre.com, a.hajda@samsung.com,
	s.nawrocki@samsung.com, galak@codeaurora.org, olof@lixom.net,
	Alison_Chaiken@mentor.com, linux-arm-kernel@lists.infradead.org,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Tue, 5 Nov 2013 19:48:11 +0100	[thread overview]
Message-ID: <201311051948.11992.arnd@arndb.de> (raw)
In-Reply-To: <20131105180314.GB14706@obsidianresearch.com>

On Tuesday 05 November 2013, Jason Gunthorpe wrote:
> I've been brewing a patch set to fix TPM, but it is a large effort. I
> would think other legacy subsystems that are not fully class device
> enabled will have similar challenges?

I think a common pattern is to have the subsystem data be referenced from
the driver data using a pointer, and contain a back reference to the struct
device, which should not be a problem.

> How about an alternate entry point:
> 
>  net = alloc_etherdev(sizeof(foo_priv));
>  priv = netdev_priv(net);
>  devm_probe_priv(priv, probes);
> 
> Don't touch the drvdata for that flow.

Yes, that would work, but it would prevent another idea I had that I haven't
mentioned here: If we add a pointer to the 'struct devm_probe' array to
'struct device_driver', those can actually be called by the driver core
code before entering the probe() callback, or potentially replace the
probe() function entirely for simple drivers (provided we add a few more
bits that we haven't talked about here.

I was originally thinking of something like

int devm_probe_alloc_netdev(struct device *dev, struct devm_probe *probe)
{
	struct net_device *netdev, **netdevp;

	/* open-coded devm_alloc_ethernet */
	netdevp = devres_alloc(devm_free_netdev, sizeof (*netdevp), GFP_KERNEL);
	if (!netdevp)
		return -ENOMEM;

	netdev = alloc_etherdev(probe->size);
	if (!netdev) {
		devres_free(netdevp);
		return -ENOMEM;
	}

	*netdevp = netdev;
	devres_add(dev, netdevp);

	dev_set_drvdata(dev, netdev_priv(netdev));

        netdevp = dev_get_drvdata(dev) + probe->offset;
        *netdevp = netdev;
}
#define DEVM_ALLOC_NETDEV(_struct, _member) {				\
	.initfunc = devm_probe_alloc_netdev,				\
	.size = sizeof(struct _struct),					\
	.offset = offsetof_t(struct _struct, _member, struct netdev *), \
}

which would fit in nicely with the rest of the design, but now I'm no longer
sure if that would actually work with the lifetime rules of the netdev, which
would pin the refcount on the 'struct device', which in turn could prevent
the devres cleanup to be executed.

	Arnd

  reply	other threads:[~2013-11-05 18:48 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-24 21:51 [RFC PATCH dtc] C-based DT schema checker integrated into dtc Stephen Warren
2013-10-24 21:51 ` Stephen Warren
2013-10-24 23:43 ` Grant Likely
2013-10-24 23:43   ` Grant Likely
2013-10-25  4:00   ` Kumar Gala
2013-10-25  4:00     ` Kumar Gala
2013-10-25 14:44   ` Stephen Warren
2013-10-25 14:44     ` Stephen Warren
2013-10-25 15:21     ` Jon Loeliger
2013-10-25 15:21       ` Jon Loeliger
2013-10-25 17:38       ` Rob Herring
2013-10-25 17:38         ` Rob Herring
2013-10-25 23:11       ` David Gibson
2013-10-25 23:11         ` David Gibson
2013-11-03 23:15         ` Tomasz Figa
2013-11-03 23:15           ` Tomasz Figa
2013-11-03 23:26           ` Tomasz Figa
2013-11-03 23:26             ` Tomasz Figa
2013-11-04  9:28             ` Arnd Bergmann
2013-11-04  9:28               ` Arnd Bergmann
2013-11-04 12:31               ` Tomasz Figa
2013-11-04 12:31                 ` Tomasz Figa
2013-11-04 16:37               ` Stephen Warren
2013-11-04 16:37                 ` Stephen Warren
2013-11-04 18:57                 ` Olof Johansson
2013-11-04 18:57                   ` Olof Johansson
2013-11-04 20:43                 ` Arnd Bergmann
2013-11-04 20:43                   ` Arnd Bergmann
2013-11-04 21:29                   ` Jason Gunthorpe
2013-11-04 21:29                     ` Jason Gunthorpe
2013-11-04 21:43                     ` Stephen Warren
2013-11-04 21:43                       ` Stephen Warren
2013-11-04 22:21                       ` Jason Gunthorpe
2013-11-04 22:21                         ` Jason Gunthorpe
2013-11-05 12:14                         ` Arnd Bergmann
2013-11-05 12:14                           ` Arnd Bergmann
2013-11-05  8:39                     ` Arnd Bergmann
2013-11-05  8:39                       ` Arnd Bergmann
2013-11-05 18:03                       ` Jason Gunthorpe
2013-11-05 18:03                         ` Jason Gunthorpe
2013-11-05 18:48                         ` Arnd Bergmann [this message]
2013-11-05 18:48                           ` Arnd Bergmann
2013-11-05 19:12                           ` Jason Gunthorpe
2013-11-05 19:12                             ` Jason Gunthorpe
2013-11-05 19:34                             ` Arnd Bergmann
2013-11-05 19:34                               ` Arnd Bergmann
2013-11-05 19:58                               ` Jason Gunthorpe
2013-11-05 19:58                                 ` Jason Gunthorpe
2013-11-05 20:17                                 ` Arnd Bergmann
2013-11-05 20:17                                   ` Arnd Bergmann
2013-11-05 20:36                                   ` Jason Gunthorpe
2013-11-05 20:36                                     ` Jason Gunthorpe
2013-11-04 21:50                   ` Stephen Warren
2013-11-04 21:50                     ` Stephen Warren
2013-11-05  8:22                     ` Arnd Bergmann
2013-11-05  8:22                       ` Arnd Bergmann
2013-11-06 12:17                   ` Thierry Reding
2013-11-06 12:17                     ` Thierry Reding
2013-11-04 14:28           ` David Gibson
2013-11-04 14:28             ` David Gibson
2013-11-04 16:42           ` Stephen Warren
2013-11-04 16:42             ` Stephen Warren
2013-10-28 10:17     ` David Gibson
2013-10-28 10:17       ` David Gibson
2013-10-31 21:13       ` Stephen Warren
2013-10-31 21:13         ` Stephen Warren
2013-11-01 13:24         ` David Gibson
2013-11-01 13:24           ` David Gibson
2013-10-25 23:29 ` David Gibson
2013-10-25 23:29   ` David Gibson
2013-10-31 21:11   ` Stephen Warren
2013-10-31 21:11     ` Stephen Warren
2013-11-10 11:00     ` David Gibson
2013-11-10 11:00       ` David Gibson
2013-11-12 22:06       ` Stephen Warren
2013-11-12 22:06         ` Stephen Warren
2013-11-13  0:33         ` David Gibson
2013-11-13  0:33           ` David Gibson

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=201311051948.11992.arnd@arndb.de \
    --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 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.