All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suman Anna <s-anna@ti.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: Dave Gerlach <d-gerlach@ti.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	linux-arm <linux-arm-kernel@lists.infradead.org>,
	Robert Tivy <rtivy@ti.com>
Subject: Re: [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU
Date: Tue, 29 Jul 2014 11:10:17 -0500	[thread overview]
Message-ID: <53D7C769.6080500@ti.com> (raw)
In-Reply-To: <CAK=Wgbawdik3ZB0Zbeaa7eXd7M+9ev9u713GcfOc4Gy0+O80Dg@mail.gmail.com>

Hi Ohad,

On 07/29/2014 05:57 AM, Ohad Ben-Cohen wrote:
> Hi Suman,
> 
> On Tue, Jul 8, 2014 at 7:22 PM, Suman Anna <s-anna@ti.com> wrote:
>> The remoteproc driver core currently relies on iommu_present() on
>> the bus the device is on, to perform MMU management. However, this
>> logic doesn't scale for multi-arch, especially for processors that
>> do not have an IOMMU.
> 
> Is there a specific hw/scenario where you need this? Can you please
> provide more details about it?

We are trying to add a remoteproc driver for a small Cortex M3 called
the WkupM3 used for suspend/resume management on TI AM335/AM437x SoCs.
This processor does not have an MMU. Same is the case with another
processor subsystem PRU-ICSS on AM335/AM437x. All these are platform
devices, and the current iommu_present check will not scale for the same
kernel image to support OMAP4/OMAP5 and AM335/AM437x.

This patch mainly addresses the existing comments in the code,
-	 * This works for simple cases, but will easily fail with
-	 * platforms that do have an IOMMU, but not for this specific
-	 * rproc.
-	 *
-	 * This will be easily solved by introducing hw capabilities
-	 * that will be set by the remoteproc driver.

> 
> Ideally we should add them to the commit log as well.
> 
>> The individual platform implementations are required to set this
>> flag appropriately. The default setting is to not have an MMU.
> 
> Let's explicitly set the default please so this would be clear for
> users reading the code.

OK, I can update the existing drivers to explicitly set this field.

> 
>> Cc: Sjur Brændeland <sjur.brandeland@stericsson.com>
> 
> Sjur is no longer with STE, so no point in cc'ing his old email address.

Yeah, I wasn't aware until I got a bounced email.

> 
>> +       /*
>> +        * All existing OMAP IPU and DSP processors do have an MMU, and
>> +        * are expected to use MMU, so this statement suffices.
>> +        * XXX: Replace this logic if and when a need arises.
> 
> The last XXX comment is always true for any kernel code, so I'd drop it.

Sure.

regards
Suman

WARNING: multiple messages have this Message-ID (diff)
From: s-anna@ti.com (Suman Anna)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU
Date: Tue, 29 Jul 2014 11:10:17 -0500	[thread overview]
Message-ID: <53D7C769.6080500@ti.com> (raw)
In-Reply-To: <CAK=Wgbawdik3ZB0Zbeaa7eXd7M+9ev9u713GcfOc4Gy0+O80Dg@mail.gmail.com>

Hi Ohad,

On 07/29/2014 05:57 AM, Ohad Ben-Cohen wrote:
> Hi Suman,
> 
> On Tue, Jul 8, 2014 at 7:22 PM, Suman Anna <s-anna@ti.com> wrote:
>> The remoteproc driver core currently relies on iommu_present() on
>> the bus the device is on, to perform MMU management. However, this
>> logic doesn't scale for multi-arch, especially for processors that
>> do not have an IOMMU.
> 
> Is there a specific hw/scenario where you need this? Can you please
> provide more details about it?

We are trying to add a remoteproc driver for a small Cortex M3 called
the WkupM3 used for suspend/resume management on TI AM335/AM437x SoCs.
This processor does not have an MMU. Same is the case with another
processor subsystem PRU-ICSS on AM335/AM437x. All these are platform
devices, and the current iommu_present check will not scale for the same
kernel image to support OMAP4/OMAP5 and AM335/AM437x.

This patch mainly addresses the existing comments in the code,
-	 * This works for simple cases, but will easily fail with
-	 * platforms that do have an IOMMU, but not for this specific
-	 * rproc.
-	 *
-	 * This will be easily solved by introducing hw capabilities
-	 * that will be set by the remoteproc driver.

> 
> Ideally we should add them to the commit log as well.
> 
>> The individual platform implementations are required to set this
>> flag appropriately. The default setting is to not have an MMU.
> 
> Let's explicitly set the default please so this would be clear for
> users reading the code.

OK, I can update the existing drivers to explicitly set this field.

> 
>> Cc: Sjur Br?ndeland <sjur.brandeland@stericsson.com>
> 
> Sjur is no longer with STE, so no point in cc'ing his old email address.

Yeah, I wasn't aware until I got a bounced email.

> 
>> +       /*
>> +        * All existing OMAP IPU and DSP processors do have an MMU, and
>> +        * are expected to use MMU, so this statement suffices.
>> +        * XXX: Replace this logic if and when a need arises.
> 
> The last XXX comment is always true for any kernel code, so I'd drop it.

Sure.

regards
Suman

  reply	other threads:[~2014-07-29 16:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-08 16:21 [PATCH 0/2] couple of generic remoteproc enhancements Suman Anna
2014-07-08 16:21 ` Suman Anna
2014-07-08 16:22 ` [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU Suman Anna
2014-07-08 16:22   ` Suman Anna
2014-07-29 10:57   ` Ohad Ben-Cohen
2014-07-29 10:57     ` Ohad Ben-Cohen
2014-07-29 16:10     ` Suman Anna [this message]
2014-07-29 16:10       ` Suman Anna
2014-08-04 11:50       ` Ohad Ben-Cohen
2014-08-04 11:50         ` Ohad Ben-Cohen
2014-08-04 15:48         ` Suman Anna
2014-08-04 15:48           ` Suman Anna
2014-08-05  7:05           ` Ohad Ben-Cohen
2014-08-05  7:05             ` Ohad Ben-Cohen
2014-07-08 16:22 ` [PATCH 2/2] remoteproc: add support to handle internal memories Suman Anna
2014-07-08 16:22   ` Suman Anna
2014-07-29 11:00   ` Ohad Ben-Cohen
2014-07-29 11:00     ` Ohad Ben-Cohen
2014-07-29 19:33     ` Suman Anna
2014-07-29 19:33       ` Suman Anna
2014-08-19  9:10       ` Ohad Ben-Cohen
2014-08-19  9:10         ` Ohad Ben-Cohen
2014-09-15 19:39         ` Suman Anna
2014-09-15 19:39           ` Suman Anna
2014-09-23 14:16           ` Ohad Ben-Cohen
2014-09-23 14:16             ` Ohad Ben-Cohen
2014-09-23 16:42             ` Suman Anna
2014-09-23 16:42               ` Suman Anna

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=53D7C769.6080500@ti.com \
    --to=s-anna@ti.com \
    --cc=d-gerlach@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=ohad@wizery.com \
    --cc=rtivy@ti.com \
    /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.