All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Srinivas Kandagatla
	<srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
	sdharia-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	bp-l3A5Bk7waGM@public.gmane.org,
	poeschel-Xtl8qvBWbHwb1SvskN2V4Q@public.gmane.org,
	treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	andreas.noever-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	daniel-/w4YWyX8dFk@public.gmane.org,
	jkosina-AlSwsSmVLrQ@public.gmane.org,
	sharon.dvir1-MQgwKvJRKlGYZoqfULhbRA@public.gmane.org,
	joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org,
	michael.opdenacker-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kheitke-hxvC4TZJLZFWk0Htik3J/w@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org
Subject: Re: [Patch v6 1/7] slimbus: Device management on SLIMbus
Date: Wed, 1 Nov 2017 16:08:06 -0700	[thread overview]
Message-ID: <20171101230806.GD28761@minitux> (raw)
In-Reply-To: <1a1d2777-be69-98ca-afba-0ffd0e3dd80f-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Wed 18 Oct 09:38 PDT 2017, Srinivas Kandagatla wrote:
> On 17/10/17 07:23, Bjorn Andersson wrote:
> > On Fri 06 Oct 08:51 PDT 2017, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
[..]
> > > +static int slim_device_remove(struct device *dev)
> > > +{
> > > +	struct slim_device *sbdev;
> > > +	struct slim_driver *sbdrv;
> > > +	int status = 0;
> > > +
> > > +	sbdev = to_slim_device(dev);
> > > +	if (!dev->driver)
> > > +		return 0;
> > > +
> > > +	sbdrv = to_slim_driver(dev->driver);
> > > +	if (sbdrv->remove)
> > > +		status = sbdrv->remove(sbdev);
> > > +
> > > +	mutex_lock(&sbdev->report_lock);
> > > +	sbdev->notified = false;
> > > +	if (status == 0)
> > > +		sbdev->driver = NULL;
> > > +	mutex_unlock(&sbdev->report_lock);
> > > +	return status;
> > 
> > device_unregister() will call device_del() which will end up in
> > __device_release_driver() which will call this function. Upon returning
> > from this function the core expect the bus to have cleaned up after the
> > dev (normally by calling drv->remove(dev)).
> > 
> > It will completely ignore the return value and continue tearing down the
> > rest of the core resources, e.g. three lines down it will
> > devres_release_all().
> > 
> > 
> > So you have the option of sleeping, while waiting for stuff to be
> > aborted/finished and then you need to clean things up.
> > 
> > The slim_device object itself will stick around until all references are
> > dropped though.
> 
> So you are suggesting that we make slim_driver remove not return anything?
> 

Yes.

Having the opportunity of failing remove() causes driver writers to
expect that they can fail, with the result of things not being torn
down properly.

Note that right after remove() returns devm_* resources will be
released, so anything that is not torn down and for some reason later
access allocated resources would cause issues.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: gregkh@linuxfoundation.org, broonie@kernel.org,
	alsa-devel@alsa-project.org, sdharia@codeaurora.org, bp@suse.de,
	poeschel@lemonage.de, treding@nvidia.com,
	gong.chen@linux.intel.com, andreas.noever@gmail.com,
	alan@linux.intel.com, mathieu.poirier@linaro.org,
	daniel@ffwll.ch, jkosina@suse.cz, sharon.dvir1@mail.huji.ac.il,
	joe@perches.com, davem@davemloft.net, james.hogan@imgtec.com,
	michael.opdenacker@free-electrons.com, robh+dt@kernel.org,
	pawel.moll@arm.com, mark.rutland@arm.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	kheitke@audience.com, linux-arm-msm@vger.kernel.org,
	arnd@arndb.de
Subject: Re: [Patch v6 1/7] slimbus: Device management on SLIMbus
Date: Wed, 1 Nov 2017 16:08:06 -0700	[thread overview]
Message-ID: <20171101230806.GD28761@minitux> (raw)
In-Reply-To: <1a1d2777-be69-98ca-afba-0ffd0e3dd80f@linaro.org>

On Wed 18 Oct 09:38 PDT 2017, Srinivas Kandagatla wrote:
> On 17/10/17 07:23, Bjorn Andersson wrote:
> > On Fri 06 Oct 08:51 PDT 2017, srinivas.kandagatla@linaro.org wrote:
[..]
> > > +static int slim_device_remove(struct device *dev)
> > > +{
> > > +	struct slim_device *sbdev;
> > > +	struct slim_driver *sbdrv;
> > > +	int status = 0;
> > > +
> > > +	sbdev = to_slim_device(dev);
> > > +	if (!dev->driver)
> > > +		return 0;
> > > +
> > > +	sbdrv = to_slim_driver(dev->driver);
> > > +	if (sbdrv->remove)
> > > +		status = sbdrv->remove(sbdev);
> > > +
> > > +	mutex_lock(&sbdev->report_lock);
> > > +	sbdev->notified = false;
> > > +	if (status == 0)
> > > +		sbdev->driver = NULL;
> > > +	mutex_unlock(&sbdev->report_lock);
> > > +	return status;
> > 
> > device_unregister() will call device_del() which will end up in
> > __device_release_driver() which will call this function. Upon returning
> > from this function the core expect the bus to have cleaned up after the
> > dev (normally by calling drv->remove(dev)).
> > 
> > It will completely ignore the return value and continue tearing down the
> > rest of the core resources, e.g. three lines down it will
> > devres_release_all().
> > 
> > 
> > So you have the option of sleeping, while waiting for stuff to be
> > aborted/finished and then you need to clean things up.
> > 
> > The slim_device object itself will stick around until all references are
> > dropped though.
> 
> So you are suggesting that we make slim_driver remove not return anything?
> 

Yes.

Having the opportunity of failing remove() causes driver writers to
expect that they can fail, with the result of things not being torn
down properly.

Note that right after remove() returns devm_* resources will be
released, so anything that is not torn down and for some reason later
access allocated resources would cause issues.

Regards,
Bjorn

  parent reply	other threads:[~2017-11-01 23:08 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06 15:51 [Patch v6 0/7] Introduce framework for SLIMbus device drivers srinivas.kandagatla
2017-10-06 15:51 ` [Patch v6 1/7] slimbus: Device management on SLIMbus srinivas.kandagatla
2017-10-06 15:51   ` srinivas.kandagatla
2017-10-10 10:05   ` Charles Keepax
2017-10-10 10:05     ` [alsa-devel] " Charles Keepax
2017-10-10 12:34     ` Srinivas Kandagatla
2017-10-10 12:34       ` [alsa-devel] " Srinivas Kandagatla
2017-10-10 12:56       ` Charles Keepax
2017-10-10 12:56         ` [alsa-devel] " Charles Keepax
2017-10-11 10:23       ` Mark Brown
2017-10-11 10:23         ` [alsa-devel] " Mark Brown
     [not found]   ` <20171006155136.4682-2-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-10-07  4:14     ` Jonathan Neuschäfer
2017-10-07  4:14       ` Jonathan Neuschäfer
2017-10-07 10:24       ` Srinivas Kandagatla
2017-10-07 10:24         ` Srinivas Kandagatla
2017-10-10 10:45     ` [alsa-devel] " Vinod Koul
2017-10-10 10:45       ` Vinod Koul
2017-10-10 12:34       ` Srinivas Kandagatla
2017-10-10 16:49         ` Vinod Koul
2017-10-10 16:49           ` [alsa-devel] " Vinod Koul
2017-10-10 17:21           ` Srinivas Kandagatla
2017-10-11  4:07             ` Vinod Koul
2017-10-11  9:42               ` Srinivas Kandagatla
2017-10-11 10:21                 ` Vinod Koul
2017-10-11 11:23                   ` Srinivas Kandagatla
2017-10-13 19:26     ` Rob Herring
2017-10-13 19:26       ` Rob Herring
2017-10-16  9:28       ` Srinivas Kandagatla
2017-10-16  9:28         ` Srinivas Kandagatla
2017-10-17  6:23     ` Bjorn Andersson
2017-10-17  6:23       ` Bjorn Andersson
2017-10-18 16:38       ` Srinivas Kandagatla
2017-10-18 16:38         ` Srinivas Kandagatla
     [not found]         ` <1a1d2777-be69-98ca-afba-0ffd0e3dd80f-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-11-01 23:08           ` Bjorn Andersson [this message]
2017-11-01 23:08             ` Bjorn Andersson
2017-10-25  0:16     ` Stephen Boyd
2017-10-25  0:16       ` Stephen Boyd
2017-10-12 11:01   ` [alsa-devel] " Sanyog Kale
2017-10-12 13:26     ` Srinivas Kandagatla
2017-10-12 13:26       ` Srinivas Kandagatla
2017-10-23  9:06   ` Mark Brown
2017-10-06 15:51 ` [Patch v6 3/7] slimbus: qcom: Add Qualcomm Slimbus controller driver srinivas.kandagatla
2017-10-07  7:45   ` Jonathan Neuschäfer
2017-10-07 10:24     ` Srinivas Kandagatla
2017-10-07 10:24       ` Srinivas Kandagatla
2017-10-13 19:17   ` Rob Herring
2017-10-16  9:28     ` Srinivas Kandagatla
2017-10-18  7:27   ` Bjorn Andersson
2017-10-18 16:39     ` Srinivas Kandagatla
2017-10-06 15:51 ` [Patch v6 4/7] slimbus: Add support for 'clock-pause' feature srinivas.kandagatla
2017-10-07  8:06   ` Jonathan Neuschäfer
2017-10-07 10:24     ` Srinivas Kandagatla
     [not found] ` <20171006155136.4682-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-10-06 15:51   ` [Patch v6 2/7] slimbus: Add messaging APIs to slimbus framework srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
2017-10-06 15:51     ` srinivas.kandagatla
2017-10-07  6:42     ` Jonathan Neuschäfer
2017-10-07 10:24       ` Srinivas Kandagatla
2017-10-07 10:24         ` Srinivas Kandagatla
2017-10-07 12:29         ` Jonathan Neuschäfer
2017-10-10 12:19     ` Charles Keepax
2017-10-10 12:19       ` [alsa-devel] " Charles Keepax
2017-10-10 13:01       ` Srinivas Kandagatla
2017-10-10 13:01         ` [alsa-devel] " Srinivas Kandagatla
2017-10-11  4:38     ` Vinod Koul
2017-10-11  7:53       ` Arnd Bergmann
2017-10-11  7:53         ` Arnd Bergmann
2017-10-11  9:42       ` Srinivas Kandagatla
2017-10-11  9:42         ` [alsa-devel] " Srinivas Kandagatla
     [not found]         ` <aa117cb8-ba59-894c-5a82-1b38facfa841-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-10-11 10:24           ` Vinod Koul
2017-10-11 10:24             ` Vinod Koul
2017-10-11 11:12             ` Srinivas Kandagatla
2017-10-18  6:15     ` Bjorn Andersson
2017-10-18 16:39       ` Srinivas Kandagatla
2017-10-20  5:00         ` Bjorn Andersson
2017-10-20  5:00           ` Bjorn Andersson
2017-10-06 15:51   ` [Patch v6 5/7] slimbus: qcom: Add runtime-pm support using clock-pause feature srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
2017-10-06 15:51     ` srinivas.kandagatla
2017-10-07  8:22     ` Jonathan Neuschäfer
2017-10-07 10:25       ` Srinivas Kandagatla
2017-10-06 15:51   ` [Patch v6 7/7] MAINTAINERS: Add SLIMbus maintainer srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
2017-10-06 15:51     ` srinivas.kandagatla
2017-10-20  5:00     ` Bjorn Andersson
2017-10-20  5:00       ` Bjorn Andersson
2017-10-06 15:51 ` [Patch v6 6/7] regmap: add SLIMBUS support srinivas.kandagatla
2017-10-07  5:02   ` Jonathan Neuschäfer
2017-10-07 10:25     ` Srinivas Kandagatla
2017-10-20  5:00   ` Bjorn Andersson
2017-10-20  5:00     ` Bjorn Andersson
2017-10-25  0:43 [Patch v6 1/7] slimbus: Device management on SLIMbus Rishi Bhattacharya
2017-10-25  0:43 ` Rishi Bhattacharya

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=20171101230806.GD28761@minitux \
    --to=bjorn.andersson-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \
    --cc=alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org \
    --cc=andreas.noever-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=bp-l3A5Bk7waGM@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=daniel-/w4YWyX8dFk@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org \
    --cc=jkosina-AlSwsSmVLrQ@public.gmane.org \
    --cc=joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org \
    --cc=kheitke-hxvC4TZJLZFWk0Htik3J/w@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=michael.opdenacker-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=poeschel-Xtl8qvBWbHwb1SvskN2V4Q@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sdharia-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=sharon.dvir1-MQgwKvJRKlGYZoqfULhbRA@public.gmane.org \
    --cc=srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.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.