From: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Subject: Re: [PATCH] of: use platform_device_add
Date: Wed, 21 Nov 2012 10:34:03 -0800 [thread overview]
Message-ID: <20121121183403.GA7657@kroah.com> (raw)
In-Reply-To: <1353521759-28263-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
On Wed, Nov 21, 2012 at 06:15:59PM +0000, Grant Likely wrote:
> This allows platform_device_add a chance to call insert_resource on all
> of the resources from OF. At a minimum this fills in proc/iomem and
> presumably makes resource tracking and conflict detection work better.
> However, it has the side effect of moving all OF generated platform
> devices from /sys/devices to /sys/devices/platform/. It /shouldn't/
> break userspace because userspace is not supposed to depend on the full
> path (because userspace always does what it is supposed to, right?).
>
> It also has a backup call to of_device_add() when running on PowerPC to
> catch any devices that have overlapping regions. It will complain about
> them, but it will not fail to register the device.
>
> Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> ---
>
> Greg, do you mind taking a look at this? The reason the OF code hasn't been
> calling platform_device_add() directly to this point is:
> a) there are some trees with resource overlays
> b) I want the devices in /sys/devices not /sys/devices/platform.
Putting the devices all in the "flat" location of /sys/devices/ is a bit
worrisome to me. What's wrong with platform/ ? That is what they are,
right? Why change this?
> I could easily add exceptions to platform_device_add() for both those cases, but
> I don't like adding DT exceptions to the common code. However, I still need to
> support the platforms that unfortunately have overlapping resources. This patch
> does that by still calling the old path if platform_device_add() fails, but it
> isn't nice either because of_device_add() has to duplicate
> platform_device_add(). Blech. Plus the exception only applies for PowerPC.
>
> So, how do you feel about having a 'relaxed' mode for platform_device_add()
> which means it won't fail if resources overlap and maybe won't do the silly
> platform_bus parent thing. Thoughts?
I have no objection for the resource issue, if you assure me it will not
be abused :)
But the sysfs location is still an issue, sorry.
thanks,
greg k-h
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linux-kernel@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org,
Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Rob Herring <rob.herring@calxeda.com>
Subject: Re: [PATCH] of: use platform_device_add
Date: Wed, 21 Nov 2012 10:34:03 -0800 [thread overview]
Message-ID: <20121121183403.GA7657@kroah.com> (raw)
In-Reply-To: <1353521759-28263-1-git-send-email-grant.likely@secretlab.ca>
On Wed, Nov 21, 2012 at 06:15:59PM +0000, Grant Likely wrote:
> This allows platform_device_add a chance to call insert_resource on all
> of the resources from OF. At a minimum this fills in proc/iomem and
> presumably makes resource tracking and conflict detection work better.
> However, it has the side effect of moving all OF generated platform
> devices from /sys/devices to /sys/devices/platform/. It /shouldn't/
> break userspace because userspace is not supposed to depend on the full
> path (because userspace always does what it is supposed to, right?).
>
> It also has a backup call to of_device_add() when running on PowerPC to
> catch any devices that have overlapping regions. It will complain about
> them, but it will not fail to register the device.
>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> Greg, do you mind taking a look at this? The reason the OF code hasn't been
> calling platform_device_add() directly to this point is:
> a) there are some trees with resource overlays
> b) I want the devices in /sys/devices not /sys/devices/platform.
Putting the devices all in the "flat" location of /sys/devices/ is a bit
worrisome to me. What's wrong with platform/ ? That is what they are,
right? Why change this?
> I could easily add exceptions to platform_device_add() for both those cases, but
> I don't like adding DT exceptions to the common code. However, I still need to
> support the platforms that unfortunately have overlapping resources. This patch
> does that by still calling the old path if platform_device_add() fails, but it
> isn't nice either because of_device_add() has to duplicate
> platform_device_add(). Blech. Plus the exception only applies for PowerPC.
>
> So, how do you feel about having a 'relaxed' mode for platform_device_add()
> which means it won't fail if resources overlap and maybe won't do the silly
> platform_bus parent thing. Thoughts?
I have no objection for the resource issue, if you assure me it will not
be abused :)
But the sysfs location is still an issue, sorry.
thanks,
greg k-h
next prev parent reply other threads:[~2012-11-21 18:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-21 18:15 [PATCH] of: use platform_device_add Grant Likely
2012-11-21 18:15 ` Grant Likely
[not found] ` <1353521759-28263-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2012-11-21 18:34 ` Greg Kroah-Hartman [this message]
2012-11-21 18:34 ` Greg Kroah-Hartman
[not found] ` <20121121183403.GA7657-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2012-11-22 21:19 ` Grant Likely
2012-11-22 21:19 ` Grant Likely
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=20121121183403.GA7657@kroah.com \
--to=gregkh-hqyy1w1ycw8ekmwlsbkhg0b+6bgklq7r@public.gmane.org \
--cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
--cc=grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \
--cc=jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@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.