All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pawel Moll <pawel.moll@arm.com>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: "paul@pwsan.com" <paul@pwsan.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"arm@kernel.org" <arm@kernel.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Olof Johansson <olof@lixom.net>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 4/5] [SCSI] Do not use platform_bus as a parent
Date: Fri, 25 Jul 2014 16:40:56 +0100	[thread overview]
Message-ID: <1406302856.17726.18.camel@hornet> (raw)
In-Reply-To: <1406299616.1789.13.camel@jarvis.lan>

On Fri, 2014-07-25 at 15:46 +0100, James Bottomley wrote:
> On Fri, 2014-07-25 at 15:23 +0100, Pawel Moll wrote:
> > The host devices without a parent were "forcefully adopted"
> > by platform bus. This patch removes this assignment. In
> > effect the dev_dev may be NULL now, which means ISA.
> > 
> > Cc: James E.J. Bottomley <JBottomley@parallels.com>
> > Cc: linux-scsi@vger.kernel.org
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > ---
> > 
> > This patch is a part of effort to remove references to platform_bus
> > and make it static.
> > 
> > James, could you please have a look and advice if the change is
> > correct? Would you happen to know the "real reasons" behind
> > using the root platform_bus device a parent?
> 
> Yes, for DMA purposes, the parent cannot now be NULL; we'll get a panic
> in the DMA transfers if it is.  

That's what I though at the beginning as well, but then I crawled
through get_dma_ops(struct device *dev) and it seems that on most
architectures (all but two, if I remember correctly) it will return a
default set of DMA ops if the dev == NULL, eg.

static inline struct dma_map_ops *get_dma_ops(struct device *dev)
{
#ifndef CONFIG_X86_DEV_DMA_OPS
        return dma_ops;
#else
        if (unlikely(!dev) || !dev->archdata.dma_ops)
                return dma_ops;
        else
                return dev->archdata.dma_ops;
#endif 
}

in arch/x86/include/asm/dma-mapping.h. Now, there seem to be only a
handful of places where dev_dma is dereferenced: scsi_dma_map and
scsi_dma_unmap in drivers/scsi/scsi_lib_dma.c, where all the calls seem
to be "NULL resistant" and __scsi_alloc_queue in __scsi_alloc_queue
which will oops as you said.

Anyway, if you are saying that dev_dma must not be NULL at any
circumstances, I'll either have to find some kind of replacement for
platform_bus or convince Greg that platform_bus must not be made
static ;-)

> A lot of the legacy ISA device on x86
> and I thought some ARM SOC devices don't pass in the parent device, so
> we hang them off a known parent.

That's another thing I'm not sure - once assigned, is the dma_dev
related to the parent in any way? Even more - is the shost_gendev.parent
used at all? Doesn't seem to be. If it's only about a place in the
device model hierarchy, leaving parent as NULL will make such device a
virtual one, which it probably should be...

Paweł


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: pawel.moll@arm.com (Pawel Moll)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/5] [SCSI] Do not use platform_bus as a parent
Date: Fri, 25 Jul 2014 16:40:56 +0100	[thread overview]
Message-ID: <1406302856.17726.18.camel@hornet> (raw)
In-Reply-To: <1406299616.1789.13.camel@jarvis.lan>

On Fri, 2014-07-25 at 15:46 +0100, James Bottomley wrote:
> On Fri, 2014-07-25 at 15:23 +0100, Pawel Moll wrote:
> > The host devices without a parent were "forcefully adopted"
> > by platform bus. This patch removes this assignment. In
> > effect the dev_dev may be NULL now, which means ISA.
> > 
> > Cc: James E.J. Bottomley <JBottomley@parallels.com>
> > Cc: linux-scsi at vger.kernel.org
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > ---
> > 
> > This patch is a part of effort to remove references to platform_bus
> > and make it static.
> > 
> > James, could you please have a look and advice if the change is
> > correct? Would you happen to know the "real reasons" behind
> > using the root platform_bus device a parent?
> 
> Yes, for DMA purposes, the parent cannot now be NULL; we'll get a panic
> in the DMA transfers if it is.  

That's what I though at the beginning as well, but then I crawled
through get_dma_ops(struct device *dev) and it seems that on most
architectures (all but two, if I remember correctly) it will return a
default set of DMA ops if the dev == NULL, eg.

static inline struct dma_map_ops *get_dma_ops(struct device *dev)
{
#ifndef CONFIG_X86_DEV_DMA_OPS
        return dma_ops;
#else
        if (unlikely(!dev) || !dev->archdata.dma_ops)
                return dma_ops;
        else
                return dev->archdata.dma_ops;
#endif 
}

in arch/x86/include/asm/dma-mapping.h. Now, there seem to be only a
handful of places where dev_dma is dereferenced: scsi_dma_map and
scsi_dma_unmap in drivers/scsi/scsi_lib_dma.c, where all the calls seem
to be "NULL resistant" and __scsi_alloc_queue in __scsi_alloc_queue
which will oops as you said.

Anyway, if you are saying that dev_dma must not be NULL at any
circumstances, I'll either have to find some kind of replacement for
platform_bus or convince Greg that platform_bus must not be made
static ;-)

> A lot of the legacy ISA device on x86
> and I thought some ARM SOC devices don't pass in the parent device, so
> we hang them off a known parent.

That's another thing I'm not sure - once assigned, is the dma_dev
related to the parent in any way? Even more - is the shost_gendev.parent
used at all? Doesn't seem to be. If it's only about a place in the
device model hierarchy, leaving parent as NULL will make such device a
virtual one, which it probably should be...

Pawe?

WARNING: multiple messages have this Message-ID (diff)
From: Pawel Moll <pawel.moll@arm.com>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Olof Johansson <olof@lixom.net>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	"paul@pwsan.com" <paul@pwsan.com>, Arnd Bergmann <arnd@arndb.de>,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	"arm@kernel.org" <arm@kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH 4/5] [SCSI] Do not use platform_bus as a parent
Date: Fri, 25 Jul 2014 16:40:56 +0100	[thread overview]
Message-ID: <1406302856.17726.18.camel@hornet> (raw)
In-Reply-To: <1406299616.1789.13.camel@jarvis.lan>

On Fri, 2014-07-25 at 15:46 +0100, James Bottomley wrote:
> On Fri, 2014-07-25 at 15:23 +0100, Pawel Moll wrote:
> > The host devices without a parent were "forcefully adopted"
> > by platform bus. This patch removes this assignment. In
> > effect the dev_dev may be NULL now, which means ISA.
> > 
> > Cc: James E.J. Bottomley <JBottomley@parallels.com>
> > Cc: linux-scsi@vger.kernel.org
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > ---
> > 
> > This patch is a part of effort to remove references to platform_bus
> > and make it static.
> > 
> > James, could you please have a look and advice if the change is
> > correct? Would you happen to know the "real reasons" behind
> > using the root platform_bus device a parent?
> 
> Yes, for DMA purposes, the parent cannot now be NULL; we'll get a panic
> in the DMA transfers if it is.  

That's what I though at the beginning as well, but then I crawled
through get_dma_ops(struct device *dev) and it seems that on most
architectures (all but two, if I remember correctly) it will return a
default set of DMA ops if the dev == NULL, eg.

static inline struct dma_map_ops *get_dma_ops(struct device *dev)
{
#ifndef CONFIG_X86_DEV_DMA_OPS
        return dma_ops;
#else
        if (unlikely(!dev) || !dev->archdata.dma_ops)
                return dma_ops;
        else
                return dev->archdata.dma_ops;
#endif 
}

in arch/x86/include/asm/dma-mapping.h. Now, there seem to be only a
handful of places where dev_dma is dereferenced: scsi_dma_map and
scsi_dma_unmap in drivers/scsi/scsi_lib_dma.c, where all the calls seem
to be "NULL resistant" and __scsi_alloc_queue in __scsi_alloc_queue
which will oops as you said.

Anyway, if you are saying that dev_dma must not be NULL at any
circumstances, I'll either have to find some kind of replacement for
platform_bus or convince Greg that platform_bus must not be made
static ;-)

> A lot of the legacy ISA device on x86
> and I thought some ARM SOC devices don't pass in the parent device, so
> we hang them off a known parent.

That's another thing I'm not sure - once assigned, is the dma_dev
related to the parent in any way? Even more - is the shost_gendev.parent
used at all? Doesn't seem to be. If it's only about a place in the
device model hierarchy, leaving parent as NULL will make such device a
virtual one, which it probably should be...

Paweł


  reply	other threads:[~2014-07-25 15:40 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-25 14:23 [PATCH 1/5] ARM: imx: Remove references to platform_bus in mxc code Pawel Moll
2014-07-25 14:23 ` Pawel Moll
2014-07-25 14:23 ` Pawel Moll
2014-07-25 14:23 ` [PATCH 2/5] char: tile-srom: Remove reference to platform_bus Pawel Moll
2014-07-25 14:23   ` Pawel Moll
     [not found]   ` <1406298233-27876-2-git-send-email-pawel.moll-5wv7dgnIgG8@public.gmane.org>
2014-07-31 20:24     ` Chris Metcalf
2014-07-31 20:24       ` Chris Metcalf
2014-07-31 20:24       ` Chris Metcalf
2014-07-31 21:32       ` Greg Kroah-Hartman
2014-07-31 21:32         ` Greg Kroah-Hartman
     [not found]       ` <53DAA605.2030500-kv+TWInifGbQT0dZR+AlfA@public.gmane.org>
2014-08-01 17:21         ` Pawel Moll
2014-08-01 17:21           ` Pawel Moll
2014-08-01 17:21           ` Pawel Moll
2014-08-05 20:08           ` Chris Metcalf
2014-08-05 20:08             ` Chris Metcalf
2014-08-05 20:08             ` Chris Metcalf
     [not found]             ` <53E139C8.9000502-kv+TWInifGbQT0dZR+AlfA@public.gmane.org>
2014-08-05 23:06               ` Greg Kroah-Hartman
2014-08-05 23:06                 ` Greg Kroah-Hartman
2014-08-05 23:06                 ` Greg Kroah-Hartman
2014-08-08 16:34               ` Pawel Moll
2014-08-08 16:34                 ` Pawel Moll
2014-08-08 16:34                 ` Pawel Moll
2014-08-08 16:39                 ` Pawel Moll
2014-08-08 16:39                   ` Pawel Moll
2014-08-11  2:38                 ` Chris Metcalf
2014-08-11  2:38                   ` Chris Metcalf
2014-08-11  2:38                   ` Chris Metcalf
2014-08-29 18:43                 ` Chris Metcalf
2014-08-29 18:43                   ` Chris Metcalf
2014-08-29 18:43                   ` Chris Metcalf
     [not found]                   ` <5400C9C1.4060904-kv+TWInifGbQT0dZR+AlfA@public.gmane.org>
2014-09-01 12:27                     ` Pawel Moll
2014-09-01 12:27                       ` Pawel Moll
2014-09-01 12:27                       ` Pawel Moll
2014-09-01 13:53                       ` Chris Metcalf
2014-09-01 13:53                         ` Chris Metcalf
2014-09-01 13:53                         ` Chris Metcalf
2014-07-25 14:23 ` [PATCH 3/5] mmc: sdhci-pltfm: Do not use parent as the host's device Pawel Moll
2014-07-25 14:23   ` Pawel Moll
2014-07-25 14:23   ` Pawel Moll
2014-08-08 16:36   ` Pawel Moll
2014-08-08 16:36     ` Pawel Moll
2014-08-08 16:36     ` Pawel Moll
2014-08-11  9:07     ` Ulf Hansson
2014-08-11  9:07       ` Ulf Hansson
2014-08-11  9:07       ` Ulf Hansson
     [not found]       ` <CAPDyKFoLU6pnJFmKe7CB0q-hKfwv4uCPSr0cE4aoYMvfhjMteQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-08-11  9:15         ` Pawel Moll
2014-08-11  9:15           ` Pawel Moll
2014-08-11  9:15           ` Pawel Moll
2014-08-11  9:15           ` Pawel Moll
2014-08-11  9:32           ` Ulf Hansson
2014-08-11  9:32             ` Ulf Hansson
2014-08-11  9:32             ` Ulf Hansson
     [not found]             ` <CAPDyKFruZxUtzUKM+PvsK-_qqcE4OcCaKSkRJ2_01y7TuQMGkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-08-12  8:58               ` Ulf Hansson
2014-08-12  8:58                 ` Ulf Hansson
2014-08-12  8:58                 ` Ulf Hansson
2014-08-12  8:58                 ` Ulf Hansson
2014-08-12 10:37                 ` [PATCH 3/5 v2] " Pawel Moll
2014-08-12 10:37                   ` Pawel Moll
2014-08-12 11:51                   ` Ulf Hansson
2014-08-12 11:51                     ` Ulf Hansson
2014-08-11 10:02           ` [PATCH 3/5] " Russell King - ARM Linux
2014-08-11 10:02             ` Russell King - ARM Linux
2014-08-11 10:02             ` Russell King - ARM Linux
2014-08-11 10:02             ` Russell King - ARM Linux
2014-07-25 14:23 ` [PATCH 4/5] [SCSI] Do not use platform_bus as a parent Pawel Moll
2014-07-25 14:23   ` Pawel Moll
2014-07-25 14:46   ` James Bottomley
2014-07-25 14:46     ` James Bottomley
2014-07-25 15:40     ` Pawel Moll [this message]
2014-07-25 15:40       ` Pawel Moll
2014-07-25 15:40       ` Pawel Moll
2014-07-26 20:11     ` Greg Kroah-Hartman
2014-07-26 20:11       ` Greg Kroah-Hartman
2014-07-27  3:52       ` James Bottomley
2014-07-27  3:52         ` James Bottomley
2014-07-27 15:07         ` Greg Kroah-Hartman
2014-07-27 15:07           ` Greg Kroah-Hartman
2014-08-01 17:25           ` Pawel Moll
2014-08-01 17:25             ` Pawel Moll
     [not found] ` <1406298233-27876-1-git-send-email-pawel.moll-5wv7dgnIgG8@public.gmane.org>
2014-07-25 14:23   ` [PATCH 5/5] platform: Make platform_bus device a platform device Pawel Moll
2014-07-25 14:23     ` Pawel Moll
2014-07-25 14:23     ` Pawel Moll
2014-07-26 20:12     ` Greg Kroah-Hartman
2014-07-26 20:12       ` Greg Kroah-Hartman
2014-08-01 17:21       ` Pawel Moll
2014-08-01 17:21         ` Pawel Moll
2014-07-26 20:13     ` Greg Kroah-Hartman
2014-07-26 20:13       ` Greg Kroah-Hartman
     [not found]       ` <20140726201351.GC21870-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2014-08-01 17:21         ` Pawel Moll
2014-08-01 17:21           ` Pawel Moll
2014-08-01 17:21           ` Pawel Moll
2014-07-28  1:45 ` [PATCH 1/5] ARM: imx: Remove references to platform_bus in mxc code Shawn Guo
2014-07-28  1:45   ` Shawn Guo
2014-07-28  1:45   ` Shawn Guo

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=1406302856.17726.18.camel@hornet \
    --to=pawel.moll@arm.com \
    --cc=Catalin.Marinas@arm.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=arm@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=paul@pwsan.com \
    --cc=pdeschrijver@nvidia.com \
    --cc=swarren@wwwdotorg.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.