linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: subhashj@codeaurora.org
Cc: Dolev Raviv <draviv@codeaurora.org>,
	james.bottomley@hansenpartnership.com,
	linux-scsi@vger.kernel.org, linux-scsi-owner@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, santoshsy@gmail.com,
	Sujit Reddy Thumma <sthumma@codeaurora.org>,
	Hannes Reinecke <hare@suse.de>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH/RFC V2 07/16] scsi: support well known logical units
Date: Sun, 24 Aug 2014 13:56:05 -0700	[thread overview]
Message-ID: <20140824205605.GA30004@infradead.org> (raw)
In-Reply-To: <82e763120409310e9d6b332854804858.squirrel@www.codeaurora.org>

On Fri, Aug 22, 2014 at 12:02:30AM -0000, subhashj@codeaurora.org wrote:
> >> +	/*
> >> +	 * put runtime pm reference for well-known logical units,
> >> +	 * drivers are expected to _get_* again during probe.
> >> +	 */
> >> +	if (scsi_is_wlun(sdev->lun))
> >> +		scsi_autopm_put_device(sdev);
> > Special casing the well known LUNs here seems wrong.  Shouldn't we do
> this
> > for any devices that don't get a driver attached to them?
> 
> Agreed, i will replace "if (scsi_is_wlun(sdev->lun))" check with "if
> (sdev->sdev_gendev.driver)".

I would really prefer the callers of autopm get/put to be symmetric to
issues with later than probe driver attach or similar.  Think of the
case say the CDROM or tape driver only gets loaded after we already
did a bus scan.  A quick check of the autopm code and it's underlying
implementation seems to suggest that nesting them is okay.  If that's
indeed the case I would suggest to restructure it the following way:

 - make the scsi_autopm_put_device call you add at the end of
   scsi_sysfs_add_sdev unconditional to make it balance out
   the get earlier in the function (and delete the now incorrect comment
   above the scsi_autopm_get_device call).
 - let drivers do paired get/put calls in their probe/remove methods.

If you still need any wlun changes after that please send them as a
separate patch with a detailed description on why you need it.

All in all the autopm code and it's underlying implementation is highly
confusing, so additional documentation on it would also be very welcome.

  reply	other threads:[~2014-08-24 20:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-22  0:02 [PATCH/RFC V2 07/16] scsi: support well known logical units subhashj
2014-08-24 20:56 ` Christoph Hellwig [this message]
2014-08-25  5:26   ` subhashj
  -- strict thread matches above, loose matches on Subject: below --
2014-08-14 13:30 [PATCH/RFC V2 00/16] UFS: Power managment support Dolev Raviv
2014-08-14 13:30 ` [PATCH/RFC V2 07/16] scsi: support well known logical units Dolev Raviv
2014-08-19 17:22   ` Christoph Hellwig
2014-08-21 21:18     ` Martin K. Petersen

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=20140824205605.GA30004@infradead.org \
    --to=hch@infradead.org \
    --cc=draviv@codeaurora.org \
    --cc=hare@suse.de \
    --cc=james.bottomley@hansenpartnership.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-scsi-owner@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=santoshsy@gmail.com \
    --cc=sthumma@codeaurora.org \
    --cc=subhashj@codeaurora.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).