Re: [PATCH 04/10] omap_hwspinlock: Replace "hweight_long(i & 0xf) != 1" with "!is_power_of_2(i & 0xf)"

[Ohad Ben-Cohen]

On Mon, Dec 7, 2015 at 5:03 PM, zhaoxiu.zeng  wrote:
> is_power_of_2 is simple, and faster than "hweightN(x) == 1" on most 
> architectures.

Thanks. I'm not sure that speed is a major concern here, since this
code executes only once during the lifetime of the driver. Readability
is probably more important.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/10] omap_hwspinlock: Replace "hweight_long(i & 0xf) != 1" with "!is_power_of_2(i & 0xf)"

[Ohad Ben-Cohen]

Hi,

On Sun, Dec 6, 2015 at 12:33 PM, Zhaoxiu Zeng  wrote:
>
> From: Zeng Zhaoxiu 
>
> Signed-off-by: Zeng Zhaoxiu 

Please explain why do you think we should make this change.

Btw, the original code used is_power_of_2, but we thought hweight is
more explicit so it was adopted.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remoteproc/wkup_m3: Use MODULE_DEVICE_TABLE to export alias

[Ohad Ben-Cohen]

On Wed, Sep 16, 2015 at 3:32 PM, Dave Gerlach  wrote:
> Use MODULE_DEVICE_TABLE with wkup_m3_rproc_of_match so the module alias
> is exported and the wkup_m3_rproc driver can automatically probe.
>
> Signed-off-by: Dave Gerlach 

Applied to remoteproc-next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] hwspinlock for 4.2

[Ohad Ben-Cohen]

The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:

  Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/hwspinlock.git
tags/hwspinlock-4.2

for you to fetch changes up to bd5717a4632cdecafe82d03de7dcb3b1876e2828:

  hwspinlock: qcom: Correct msb in regmap_field (2015-07-01 16:15:05 +0300)


- hwspinlock core DT support from Suman Anna
- OMAP hwspinlock DT support from Suman Anna
- QCOM hwspinlock DT support from Bjorn Andersson
- a new CSR atlas7 hwspinlock driver from Wei Chen
- CSR atlas7 hwspinlock DT binding document from Wei Chen
- a tiny QCOM hwspinlock driver fix from Bjorn Andersson


Bjorn Andersson (3):
  DT: hwspinlock: Add binding documentation for Qualcomm hwmutex
  hwspinlock: qcom: Add support for Qualcomm HW Mutex block
  hwspinlock: qcom: Correct msb in regmap_field

Suman Anna (4):
  Documentation: dt: add common bindings for hwspinlock
  hwspinlock/core: add device tree support
  Documentation: dt: add the omap hwspinlock bindings document
  hwspinlock/omap: add support for dt nodes

Wei Chen (2):
  hwspinlock: add a CSR atlas7 driver
  DT: hwspinlock: add the CSR atlas7 hwspinlock bindings document

 Documentation/devicetree/bindings/hwlock/hwlock.txt  |  59
+++
 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt |  26

 Documentation/devicetree/bindings/hwlock/qcom-hwspinlock.txt |  39
+++
 Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt |  28
+
 Documentation/hwspinlock.txt |  10 ++
 MAINTAINERS  |   1 -
 arch/arm/mach-omap2/Makefile |   3 --
 arch/arm/mach-omap2/hwspinlock.c |  60

 drivers/hwspinlock/Kconfig   |  24
+++
 drivers/hwspinlock/Makefile  |   2 ++
 drivers/hwspinlock/hwspinlock_core.c |  79
+++
 drivers/hwspinlock/omap_hwspinlock.c |  18 ---
 drivers/hwspinlock/qcom_hwspinlock.c | 181
+++
 drivers/hwspinlock/sirf_hwspinlock.c | 136

 include/linux/hwspinlock.h   |   7 +
 15 files changed, 605 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
 create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
 create mode 100644 Documentation/devicetree/bindings/hwlock/qcom-hwspinlock.txt
 create mode 100644 Documentation/devicetree/bindings/hwlock/sirf,hwspinlock.txt
 delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
 create mode 100644 drivers/hwspinlock/qcom_hwspinlock.c
 create mode 100644 drivers/hwspinlock/sirf_hwspinlock.c
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] remoteproc for 4.2

[Ohad Ben-Cohen]

The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:

  Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/remoteproc-4.2

for you to fetch changes up to 8de3dbd0895bebe52d069a82feae8e3fb51c1bdf:

  remoteproc: fix !CONFIG_OF build breakage (2015-06-18 11:44:41 +0300)


- remoteproc fixes/cleanups from Suman Anna
- new remoteproc TI Wakeup M3 driver from Dave Gerlach
- remoteproc core support for TI's Wakeup M3 driver from both Dave and Suman
- tiny remoteproc build fix from myself


Dave Gerlach (3):
  remoteproc: introduce rproc_get_by_phandle API
  Documentation: dt: add bindings for TI Wakeup M3 processor
  remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

Ohad Ben-Cohen (1):
  remoteproc: fix !CONFIG_OF build breakage

Suman Anna (4):
  remoteproc/ste: add blank lines after declarations
  remoteproc/davinci: fix quoted split string checkpatch warning
  remoteproc: fix various checkpatch warnings
  remoteproc: add a rproc ops for performing address translation

 Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt |  52
++
 Documentation/remoteproc.txt   |   6 +++
 drivers/remoteproc/Kconfig |  13 ++
 drivers/remoteproc/Makefile|   1 +
 drivers/remoteproc/da8xx_remoteproc.c  |   3 +-
 drivers/remoteproc/remoteproc_core.c   | 115
+--
 drivers/remoteproc/remoteproc_internal.h   |   2 +-
 drivers/remoteproc/ste_modem_rproc.c   |   4 +-
 drivers/remoteproc/wkup_m3_rproc.c | 257
+
 include/linux/platform_data/wkup_m3.h  |  30
+
 include/linux/remoteproc.h |   9 ++--
 11 files changed, 460 insertions(+), 32 deletions(-)
 create mode 100644
Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
 create mode 100644 drivers/remoteproc/wkup_m3_rproc.c
 create mode 100644 include/linux/platform_data/wkup_m3.h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/4] remoteproc: introduce rproc_get_by_phandle API

[Ohad Ben-Cohen]

On Thu, Jun 18, 2015 at 7:01 PM, Dave Gerlach d-gerl...@ti.com wrote:
 Oh sorry about. Pulled your for-next, tried it out, everything looks good to 
 me,
 thanks!

Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/4] remoteproc: introduce rproc_get_by_phandle API

[Ohad Ben-Cohen]

Hi Dave,

On Wed, Jun 17, 2015 at 9:46 PM, Dave Gerlach d-gerl...@ti.com wrote:
 Allow users of remoteproc the ability to get a handle to an rproc by
 passing a phandle supplied in the user's device tree node. This is
 useful in situations that require manual booting of the rproc.

 This patch uses the code removed by commit 40e575b1d0b3 (remoteproc:
 remove the get_by_name/put API) for the ref counting but is modified
 to use a simple list and locking mechanism and has rproc_get_by_name
 replaced with an rproc_get_by_phandle API.

 Signed-off-by: Dave Gerlach d-gerl...@ti.com
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
 v4-v5: Fixed build error from of_find_node_by_phandle when !CONFIG_OF
 based on offline discussion.

We can't rebase our for-next branch, so I've applied a
similar fix in a separate patch.

Please check our for-next branch to make sure things still work for you.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/4] remoteproc: Introduce wkup_m3_rproc driver

[Ohad Ben-Cohen]

On Fri, May 22, 2015 at 11:45 PM, Dave Gerlach d-gerl...@ti.com wrote:
 Hi,
 This patch series is v4 of the series to add a wkup_m3_rproc driver
 for TI AM335xi and AM437x SoCs. This family of SoCs contains an ARM
 Cortex M3 coprocessor that is responsible for low-level power tasks
 that cannot be handled by the main ARM Cortex A8 so firmware running
 from the CM3 can be used instead. This driver handles loading
 of the firmware and reset of the CM3. Change info can be found
 with each patch.

Applied all 4 patches to remoteproc's for-next branch.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] remoteproc: add a rproc ops for performing address translation

[Ohad Ben-Cohen]

Hi Dave,

On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach d-gerl...@ti.com wrote:
 From: Suman Anna s-a...@ti.com

 The rproc_da_to_va API is currently used to perform any device to
 kernel address translations to meet the different needs of the remoteproc
 core/drivers (eg: loading). The functionality is achieved within the
 remoteproc core, and is limited only for carveouts allocated within the
 core.

 A new rproc ops, da_to_va, is added to provide flexibility to platform
 implementations to perform the address translation themselves when the
 above conditions cannot be met by the implementations. The rproc_da_to_va()
 API is extended to invoke this ops if present, and fallback to regular
 processing if the platform implementation cannot provide the translation.
 This will allow any remoteproc implementations to translate addresses for
 dedicated memories like internal memories.

Can you please provide specific examples where this is needed and how
it is going to be used?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] remoteproc for 4.1

[Ohad Ben-Cohen]

The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:

  Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/remoteproc-4.1-next

for you to fetch changes up to 315491e5d6ee66838a18a8ca0c14e6ffb376e48c:

  remoteproc: add IOMMU hardware capability flag (2015-03-12 10:43:26 +0200)


Suman Anna is adding remoteproc support for processors not behind IOMMUs.


Suman Anna (1):
  remoteproc: add IOMMU hardware capability flag

 drivers/remoteproc/da8xx_remoteproc.c |  1 +
 drivers/remoteproc/omap_remoteproc.c  |  2 ++
 drivers/remoteproc/remoteproc_core.c  | 15 ++-
 drivers/remoteproc/ste_modem_rproc.c  |  1 +
 include/linux/remoteproc.h|  2 ++
 5 files changed, 8 insertions(+), 13 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/4] hwspinlock core omap dt support

[Ohad Ben-Cohen]

On Thu, Apr 16, 2015 at 12:31 PM, Mark Rutland mark.rutl...@arm.com wrote:
 Hi,

 Apologies for the delay on this.

  Gentle reminder. Can you please provide your ack on the bindings in this
  series (Patches 1  3) for Ohad to queue up the series for 4.1.
 

 Ping again, if you can provide your ack on these bindings and the qcom
 hwmutex bindings, then Ohad can pick up both the series for 4.1. Both
 series are blocked on getting the binding acks.

 Both the bindings look sane to me, so for patches 1 and 3:

 Acked-by: Mark Rutland mark.rutl...@arm.com

Great, thanks a lot Mark!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna s-a...@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. Replace this logic instead by using a h/w
 capability flag for the presence of IOMMU in the rproc structure.

 This issue is seen on OMAP platforms when 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.

 The existing platform implementation drivers - OMAP remoteproc, STE
 Modem remoteproc and DA8xx remoteproc, are updated as well to properly
 configure the newly added rproc field.

 Cc: Robert Tivy rt...@ti.com
 Cc: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Suman Anna s-a...@ti.com

Applied to remoteproc's for-next branch.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/4] hwspinlock core omap dt support

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Mar 5, 2015 at 4:01 AM, Suman Anna s-a...@ti.com wrote:
 This is the latest version of the hwspinlock dt support series,
 rebased onto v4.0-rc1 and addressing the long discussion on the
 bindings in v7 [1]. I really hope that this series can make it
 into 4.1.

From a quick glance this looks great, thanks!

 Mark,
 Can you please provide your Acked-by for the binding documents
 so that Ohad can pick up the patches for the next merge window?

That would be perfect. Once we'll have it I could move forward with
this towards 4.1.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Wed, Feb 11, 2015 at 10:57 PM, Tony Lindgren t...@atomide.com wrote:
  +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem 
  *rsc,
  +  int offset, int avail)
  +{
 ...
  +   va = (__force void *)ioremap_nocache(rsc-pa, rsc-len);

 Back in the days when we developed remoteproc there was a tremendous
 effort to move away from ioremap when not strictly needed.

 The use of ioremap in general is just fine for drivers as long
 as they access a dedicated area to the specific device. Accessing
 random registers and memory in the SoC is what I'm worried about.

Yes, the proposed interface essentially allows exactly this random
access, since the parameters to ioremap would be provided from the
user space (specifically from the resource table contained within the
firmware of the remote processor).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna s-a...@ti.com wrote:
 My original motivation was that it would only need to be added on
 firmwares requiring support for loading into internal memories,
 otherwise, these are something left to be managed by the software
 running on the remote processor completely, and MPU will not even touch
 them.

Sure. But even if you guys will use this interface correctly, this
patch essentially exposes ioremap to user space, which is something we
generally want to avoid.

 So, let me know if this is a NAK. If so, we have two options - one to go
 the sram node model where each of them have to be defined separately,
 and have a specific property in the rproc nodes to be able to get the
 gen_pool handles. The other one is simply to define these as reg and
 use devm_ioremap_resource() (so use DT for defining the regions instead
 of a resource table entry).

Any approach where these regions are defined explicitly really sounds
better. If you could look into these two alternatives that would be
great.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

On Fri, Feb 6, 2015 at 2:34 AM, Bjorn Andersson bj...@kryo.se wrote:
 Yep, will do as soon as I hear from Ohad on what to do with the patch
 hwspinlock/core: maintain a list of registered hwspinlock banks that I
 dropped from v7. Without that and dropping hwlock-base-id, I can't get
 the translations done.


 My suggestion is to replace the global id-tree with a list of hwlocks
 and iterate over these if we ever get more than one driver registered.
 As long as #hwlock-drivers  log(#total-hwlocks) this should be
 faster.

 I would however argue that clients that would notice any kind of
 difference are using the API incorrectly.

 Unfortunately this is a somewhat larger change than just slapping DT
 support on the framework :/

I suspect we want to wait with framework changes until there's a real
hardware use case justifying them.

With regard to DT, however, we obviously do want to be prepared for
multiple hwlock blocks even if they do not exist today.

So how about adopting an approach where:
- DT fully supports multi hwlock blocks (i.e. no global id field)
- Framework left mostly unchanged at the moment. This means issuing an
explicit error in case a secondary hwlock block shows up, and then
hwlock id could trivially be the lock index.

If multi hwlock hardware use case, or some new hwlock id translation
requirements, do show up in the future, it's always easy to add
framework support for it. At that point we'll know better what the
requirements are, and framework changes would be justifiable.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna s-a...@ti.com wrote:
 A remote processor may need to load certain firmware sections into
 internal memories (eg: RAM at L1 or L2 levels) for performance or
 other reasons. Introduce a new resource type (RSC_INTMEM) and add
 an associated handler function to handle such memories. The handler
 creates a kernel mapping for the resource's 'pa' (physical address).
...
 + * rproc_handle_intmem() - handle internal memory resource entry
 + * @rproc: rproc handle
 + * @rsc: the intmem resource entry
 + * @offset: offset of the resource data in resource table
 + * @avail: size of available data (for image validation)
 + *
 + * This function will handle firmware requests for mapping a memory region
 + * internal to a remote processor into kernel. It neither allocates any
 + * physical pages, nor performs any iommu mapping, as this resource entry
 + * is primarily used for representing physical internal memories. If the
 + * internal memory region can only be accessed through an iommu, please
 + * use a devmem resource entry.
 + *
 + * These resource entries should be grouped near the carveout entries in
 + * the firmware's resource table, as other firmware entries might request
 + * placing other data objects inside these memory regions (e.g. data/code
 + * segments, trace resource entries, ...).
 + */
 +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem 
 *rsc,
 +  int offset, int avail)
 +{
...
 +   va = (__force void *)ioremap_nocache(rsc-pa, rsc-len);

Back in the days when we developed remoteproc there was a tremendous
effort to move away from ioremap when not strictly needed.

I'd be happy if someone intimate with the related hardware could ack
that in this specific case ioremap is indeed needed. No need to review
the entire patch, or anything remoteproc, just make sure that
generally ioremap is how we want to access this internal memory.

Tony or Kevin any chance you could take a look and ack?

If ioremap is indeed the way to go, I'd also expect that we wouldn't
have to use __force here, but that's probably a minor patch cleanup.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Feb 3, 2015 at 10:55 PM, Suman Anna s-a...@ti.com wrote:
  On 01/09/2015 03:21 PM, Suman Anna wrote:
  Hi Ohad,
 
  The following is an updated patchset addressing the previous pending 
  comments
  from v1  v2, and are rebased onto the latest 3.19-rc3 (are rc independent
  actually).
 
  The patches are mainly developed to support the WkupM3 remote processor 
  driver
  on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
  version of Dave's WkupM3 remoteproc work [1]
 
  The only change in v3 is on the second patch, it mainly leverages the
  memcpy_toio and memset_io functions for copying/loading code into the
  internal memory sections. An additional argument has to be added to the
  rproc_da_to_va function to make this distinction.
 
  Any comments on this series, or can I assume that this will make it to
  v3.20?

 ping, want to make sure this has not fallen off your radar..

It has not. I'm a bit swamped so apologies for not taking care of this
faster. I believe I'll get to it next week.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

On Sat, Jan 31, 2015 at 7:41 AM, Ohad Ben-Cohen o...@wizery.com wrote:
 On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson bj...@kryo.se wrote:
  In a system where you have two hwlock blocks lckA and lckB, each
  consisting of 8 locks and you have dspB that can only access lckB

 This is a good example - thanks. To be able to cope with such cases we
 will have to pass a hwlock block reference and its relative lock id.

Additionally, to support such a scenario, we can no longer retain the
simple dynamic allocation API we have today, because it might end up
allocating dspB an hwlock from IckA.

We will have to make sure hwlocks are allocated only from pools
visible to the user, something that will change not only the
hwspinlock API but also the way it maintains the hwlocks.

I suspect we want to wait for such hardware to show up first, and only
then add framework support for it. Regardless, we obviously do want to
make sure our DT binding is prepared for the worse, so we'll drop the
base-id field.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

On Sat, Jan 31, 2015 at 1:29 AM, Bjorn Andersson bj...@kryo.se wrote:
 In a system where you have two hwlock blocks lckA and lckB, each
 consisting of 8 locks and you have dspB that can only access lckB

This is a good example - thanks. To be able to cope with such cases we
will have to pass a hwlock block reference and its relative lock id.

The DT binding should definitely be prepared for such cases (just kill
the base-id field?), but let's see what it means about the Linux
implementation.

Since the existence of several hwblocks is still fictional (Bjorn,
please confirm too?), we may prefer to introduce changes to support it
only when it shows up; it all depends on the amount of changes needed.
Suman, care to take a look please?

 - Sometimes a remote processor, which may not be running Linux, will
 have to dynamically allocate a hwlock, and send the ID of the
 allocated lock to us (another processor running Linux)

 I'm sorry but you cannot have a system on both sides that is allowed
 to do dynamic allocation from a limited set of resources.

Of course not. On such systems, Linux is not the one responsible for
allocating the hwlocks, at least not during part of the time or from
part of the hwlocks. There were a few different use cases, with
different semantics, that required communicating to Linux an hwlock
id, but since none of them have reached mainline, we should only
remember they may show up one day, but not put too much effort to
support them right now.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

On Tue, Jan 20, 2015 at 8:05 PM, Tony Lindgren t...@atomide.com wrote:
 How about default to Linux id space and allow overriding that with
 a module param option if needed?

I'm not sure I'm following.

If the main point of contention is the base_id field, I'm also fine
with removing it entirely, as I'm not aware of any actual user for it
(Suman please confirm?).

Mark? Rob? Will you accept Suman's patches if the base_id field is removed?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

Mark,

On Fri, Jan 16, 2015 at 12:17 PM, Mark Rutland mark.rutl...@arm.com wrote:
 The hwlock is a basic hardware primitive that allow synchronization
 between different processors in the system, which may be running Linux
 as well as other operating systems, and may have no other means of
 communication.

 The hwlock id numbers are predefined, global and static across the
 entire system: Linux may boot well after other operating systems are
 already running and using these hwlocks to communicate, and therefore,
 in order to use these hardware devices, it must not enumerate them
 differently than the rest of the system.

 That's not true.

 In order to communicate it must agree with the other users as to the
 meaning of each instance, and the protocol for use. That doesn't
 necessarily mean that Linux needs to know the numerical ID from a
 datasheet, and regardless that ID is separate from the logical ID Linux
 uses internally.

Let me describe hwspinlocks a bit more so we all get to know it better
and can then agree on a proper solution.

- What makes handling of hwspinlock ID numbers convenient is the fact
that it's not based on random datasheet numbers. In fact, hwspinlocks
is just special memory: usually datasheets just define the base
address and the size of the hwspinlock area. So any numerical ID we
use to call the locks themselves are already logical and sane, similar
to the way we handle memory (i.e. if we have 32 locks we'll always use
0..31). So hwlocks ids are very much like memory addressing, and not
irq numbers.

- Sometimes Linux will have to dynamically allocate a hwlock, and send
the ID of the allocated lock to a remote processor (which may not be
running Linux).
- Sometimes a remote processor, which may not be running Linux, will
have to dynamically allocate a hwlock, and send the ID of the
allocated lock to us (another processor running Linux)

We cannot tell in advance what kind of IPC is going to be used for
sending and receiving this hwlock ID. Some are handled by Linux
(kernel) and some by the user space. So we must be able to expose an
ID the system will understand as well as receive one.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/4] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

On Thu, Jan 15, 2015 at 4:42 PM, Rob Herring robherri...@gmail.com wrote:
 On Thu, Jan 15, 2015 at 7:55 AM, Mark Rutland mark.rutl...@arm.com wrote:
 On Thu, Jan 15, 2015 at 01:52:01PM +, Mark Rutland wrote:
 On Wed, Jan 14, 2015 at 08:58:18PM +, Suman Anna wrote:
  +- hwlock-base-id:  An unique base Id for the locks for a particular 
  hwlock
  +   device. This property is mandatory for all platform
  +   implementations.

 This property makes no sense. The ID encoded in the hwlock cells is
 relative to the instance (identified by phandle), not global. So the DT
 has no global ID space.

 Why do you think you need this?

 Having looked at the way this proeprty is used, NAK.

 If you need to carve up a Linux-internal ID space, do that dynamically.
 There is no need for this property.

 Better yet, don't create a Linux ID space for this. Everywhere we have
 one, we want to get rid of it.

Rob, Mark,

The hwlock is a basic hardware primitive that allow synchronization
between different processors in the system, which may be running Linux
as well as other operating systems, and may have no other means of
communication.

The hwlock id numbers are predefined, global and static across the
entire system: Linux may boot well after other operating systems are
already running and using these hwlocks to communicate, and therefore,
in order to use these hardware devices, it must not enumerate them
differently than the rest of the system.

Given that these id numbers are global, system-wide, static and
predefined, where Linux may just be one user of them, please
reconsider the approach as implemented by Suman, or suggest an
alternative one you may prefer.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/4] hwspinlock core omap dt support

[Ohad Ben-Cohen]

Hi Suman,

On Wed, Jan 14, 2015 at 10:58 PM, Suman Anna s-a...@ti.com wrote:
 This is an updated version of the hwspinlock dt support series,
 rebased onto v3.19-rc3 and mainly addresses the continued discussion
 on the need to maintain a list of registered spinlock banks [1].
 I have removed this patch as per your wish, and as a result the
 burden of the spinlock validation and deferred probing is pushed
 onto the hwspinlock clients. Sorry for the delay in refreshing this
 series, hopefully this can be the last revision.

 Following are the main changes in v7 w.r.t v6:
 - Dropped the patch hwspinlock/core: maintain a list of registered
   hwspinlock banks
 - Updated generic hwspinlock bindings to make hwlock-base-id property
   mandatory.
 - Updated the OMAP hwspinlock binding and DT support patches to correct
   for the mandatory hwlock-base-id property.
 - Updated the common OF helpers patch to move the of_hwspin_lock_get_base_id()
   and of_hwspin_lock_get_num_locks() functions into the internal header,
   these are no longer exported, but available for platform implementations.
   of_hwspin_lock_get_id() is also simplified now.

This looks good, thanks.

Please try to get an ACK from a DT maintainer on the first two dt
patches, and then I can take the patches to Linus.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] rpmsg for 3.19

[Ohad Ben-Cohen]

[Resending in plain text mode - sorry]

The following changes since commit 5d01410fe4d92081f349b013a2e7a95429e4f2c9:

  Linux 3.18-rc6 (2014-11-23 15:25:20 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg.git
tags/rpmsg-3.19-next

for you to fetch changes up to b1b9891441fa33fd0d49b5cb3aa7f04bca1cc1db:

  rpmsg: use less buffers when vrings are small (2014-11-26 18:24:36 +0200)


A single patch from Suman Anna which makes rpmsg
use less buffers when small vrings are being used.


Suman Anna (1):
  rpmsg: use less buffers when vrings are small

 drivers/rpmsg/virtio_rpmsg_bus.c | 44 +++-
 1 file changed, 30 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings

[Ohad Ben-Cohen]

On Thu, Nov 27, 2014 at 12:30 AM, Suman Anna s-a...@ti.com wrote:
 Yep, I have reviewed and verified the changes, it is good to go.

Applied, thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Nov 13, 2014 at 7:46 PM, Suman Anna s-a...@ti.com wrote:
 Hi Ohad,

 On 09/16/2014 01:33 PM, Suman Anna wrote:
 The buffers to be used for communication are allocated during the
 rpmsg virtio driver's probe, and the total number of buffers is
 currently hard-coded to 512. The vring configuration can vary from
 one platform to another or between different remote processors. The
 setup of the receive buffers will throw a WARN_ON if the associated
 vrings are configured with less than 256 buffers (in each direction).
 So, adjust this hard-coded value to rely on the number of buffers the
 virtqueue vring is setup with, but also limit to use 256 buffers at
 most in each direction to avoid wacky resource tables consuming up
 unreasonable memory.

 NOTE: The number of buffers is already assumed to be symmetrical
 in each direction, and that logic is not unchanged.

 Signed-off-by: Suman Anna s-a...@ti.com
 ---
 v2 changes:
 - add upper limit on buffers and update comments
 - revise patch description

 Any comments on this one, if not can you pick this up for 3.19?

Did some small changes - untested, not even compiled - can you please
make sure it works for you?

Thanks,
Ohad.
From b1b9891441fa33fd0d49b5cb3aa7f04bca1cc1db Mon Sep 17 00:00:00 2001
From: Suman Anna s-a...@ti.com
Date: Tue, 16 Sep 2014 13:33:07 -0500
Subject: [PATCH] rpmsg: use less buffers when vrings are small

Adjust the number of rpmsg buffers to rely on the size of the
vring, instead of using the hard coded value of 512 (256 per
direction).

This is needed when small vrings are being used, where 256
buffers are too much to fit in a vring.

While considering the vring size, keep using the 512 hard coded
value as an upper limit to avoid wacky resource tables consuming
unreasonable amount of memory.

NOTE: The number of buffers is already assumed to be symmetrical
in each direction, and that logic is unchanged.

Signed-off-by: Suman Anna s-a...@ti.com
[edit commit message, small code and comment simplification]
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 44 +++-
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index b6135d4..92f6af6 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -41,6 +41,7 @@
  * @svq:	tx virtqueue
  * @rbufs:	kernel address of rx buffers
  * @sbufs:	kernel address of tx buffers
+ * @num_bufs:	total number of buffers for rx and tx
  * @last_sbuf:	index of last tx buffer used
  * @bufs_dma:	dma base addr of the buffers
  * @tx_lock:	protects svq, sbufs and sleepers, to allow concurrent senders.
@@ -60,6 +61,7 @@ struct virtproc_info {
 	struct virtio_device *vdev;
 	struct virtqueue *rvq, *svq;
 	void *rbufs, *sbufs;
+	unsigned int num_bufs;
 	int last_sbuf;
 	dma_addr_t bufs_dma;
 	struct mutex tx_lock;
@@ -86,13 +88,14 @@ struct rpmsg_channel_info {
 #define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)
 
 /*
- * We're allocating 512 buffers of 512 bytes for communications, and then
- * using the first 256 buffers for RX, and the last 256 buffers for TX.
+ * We're allocating buffers of 512 bytes each for communications. The
+ * number of buffers will be computed from the number of buffers supported
+ * by the vring, upto a maximum of 512 buffers (256 in each direction).
  *
  * Each buffer will have 16 bytes for the msg header and 496 bytes for
  * the payload.
  *
- * This will require a total space of 256KB for the buffers.
+ * This will utilize a maximum total space of 256KB for the buffers.
  *
  * We might also want to add support for user-provided buffers in time.
  * This will allow bigger buffer size flexibility, and can also be used
@@ -102,9 +105,8 @@ struct rpmsg_channel_info {
  * can change this without changing anything in the firmware of the remote
  * processor.
  */
-#define RPMSG_NUM_BUFS		(512)
+#define MAX_RPMSG_NUM_BUFS	(512)
 #define RPMSG_BUF_SIZE		(512)
-#define RPMSG_TOTAL_BUF_SPACE	(RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
 
 /*
  * Local addresses are dynamically allocated on-demand.
@@ -579,7 +581,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
 	 * either pick the next unused tx buffer
 	 * (half of our buffers are used for sending messages)
 	 */
-	if (vrp-last_sbuf  RPMSG_NUM_BUFS / 2)
+	if (vrp-last_sbuf  vrp-num_bufs / 2)
 		ret = vrp-sbufs + RPMSG_BUF_SIZE * vrp-last_sbuf++;
 	/* or recycle a used one */
 	else
@@ -948,6 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	struct virtproc_info *vrp;
 	void *bufs_va;
 	int err = 0, i;
+	size_t total_buf_space;
 
 	vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
 	if (!vrp)
@@ -968,10 +971,22 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	vrp-rvq = vqs[0];
 	vrp-svq = vqs[1];
 
+	/* we expect symmetric tx/rx vrings */
+	WARN_ON(virtqueue_get_vring_size(vrp-rvq) !=
+		virtqueue_get_vring_size(vrp-svq

Re: [PATCHv6 5/5] hwspinlock/omap: add support for dt nodes

[Ohad Ben-Cohen]

Hi Bjorn,

On Thu, Nov 20, 2014 at 2:43 AM, Bjorn Andersson bj...@kryo.se wrote:
 I still have a huge problem understanding the awesomeness with the
 base_id. If you have a SoC with 2 hwlock blocks; say 8+8 locks, used
 for interaction with e.g. a modem and a video core respectively.
 Why would you in either remote system offset the locks with 8?
 Wouldn't e.g the modem use locks hwlock0:0-7 and video core use locks
 hwlock1:0-7?

Please see below

 What systems use more than one hwlock block

None that we know of. This concern was raised some time ago by Arnd
and since it was easy to deal with we added the 'base_id' notion.

 and do you know of any reasons why these hwlocks are globally numbered?

Because on an heterogeneous asymmetric multiprocessing systems you
sometimes have use cases where hwlocks are dynamically allocated and
their id numbers need to be synchronized between user space
applications, the Linux kernel, and entities running on remote
processors (which are likely not running Linux).

For this to work we have to have some system-wide global hwlocks
naming that is predefined and known to all processors. A mere number
seems like the simplest naming method. A dynamic hwlock request API
could return hwlock1:0 but a mere 8 seems easier to deal with.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

On Fri, Nov 14, 2014 at 7:09 PM, Suman Anna s-a...@ti.com wrote:
 It seems to me that hwspin_lock_request_specific failures should be
 used by clients to defer their probing. Why wouldn't such a simple
 solution work?

 Because the API always returns NULL on failures and there is no way for
 the clients to figure out if the lock id passed is invalid or the bank
 containing the lock is not registered.

It seems to me this may always be the case - lock ids may be wrong and
there's no way to fully validate them.

Let's start with the simpler approach where
hwspin_lock_request_specific failures are used by clients to defer
their probing.

If a real use case will require changes we can always do that later,
though it seems to me the only gain by changing this API is to catch a
small subset of fatal DT mistakes which will anyway be caught very
early in the development.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 5/5] hwspinlock/omap: add support for dt nodes

[Ohad Ben-Cohen]

Hi Suman,

On Wed, Nov 12, 2014 at 9:50 PM, Suman Anna s-a...@ti.com wrote:
 None of the OMAPs have multiple IP instances, and as such the base-id is
 an optional property. I have made this change to make sure we atleast
 attempt to use the value if mentioned in DT and not hard-coding the
 value to begin with (going by the optional property semantics). If and
 when multiple instances get added and a secondary node doesn't add the
 property, the node will not be registered with the core due to an
 overlap failure.

Ok, that sounds good.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Wed, Nov 12, 2014 at 9:32 PM, Suman Anna s-a...@ti.com wrote:
 Is this the validation you mentioned which requires the existence of
 hwspinlock/core: maintain a list of registered hwspinlock banks ?

 Well, not exactly. The validation is on the following segment,

 +   id = of_hwspin_lock_simple_xlate(bank, args);
 +   if (id  0 || id = bank-num_locks) {
 +   ret = -EINVAL;
 +   goto out;
 +   }

I'm not entirely convinced that this justifies adding the proposed
linked list. Can't we can get the base_id and number of locks by
traversing the DT?

 That said, it is also needed to provide the support for deferred probing
 without changing the return code conventions on the existing API.

Here too, adding a data structure maintaining memory objects and
taking care of it life cycle seems a bit overkill for anything to do
with return values.

 Yes, and wouldn't that require that the id is validated? It just cannot
 return any return value, and expect it will be verified somewhere else
 or in a following API call. Not doing the validation unnecessarily
 complicates the system usage of a lock as you are sharing an invalid
 lock to a remote processor and then you have two validation failure
 paths on different processors.

Validation is great but I'm still not convinced it can't be done
without maintaining another data structure.

Please show me specific technical issues that can't be resolved
without adding the proposed linked list.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Nov 13, 2014 at 7:38 PM, Suman Anna s-a...@ti.com wrote:
 No, not always, because, either of them can be optional between
 different platform implementations. For example, on OMAP, the number of
 locks is read from an IP register, and not coded in DT. Similarly,
 base_id can be optional on SoCs that don't have multiple IP instances.

It can, but base_id isn't optional today --- it must be provided via
platform_data --- and I'm not sure we should implicitly change this
semantics while moving to DT. We never guessed the base_id before,
even if there was only a single IP instance, and I'm not convinced we
should start doing it now.

About the number of locks - why do we even need it at this point? the
global lock id should just be base_id + the lock index.

 IMHO, this life cycle management is not that complicated

It's always very easy to add code, really. It is never complicated.
But then gradually the code becomes harder to maintain, debug, and
change.

On the contrary, achieving the same functionality with less code is
always harder. But when we get there, the results are pretty
satisfying.

In this case I still feel the extra linked list is redundant.

Please try it: ditch the hwspinlock/core: maintain a list of
registered hwspinlock banks patch, and replace of_hwspin_lock_get_id
with a slim method that just returns the base_id + the lock index. Let
me know if it works for you.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Nov 13, 2014 at 11:02 PM, Suman Anna s-a...@ti.com wrote:
 OK, lets take an example. I have say 2 device instances, say
 hwlock1: hwlock@0 {
 hwlock-num-locks = 32
 hwlock-base-id = 0;
 #hwlock-cells = 1;
 };
 hwlock2: hwlock@0 {
 hwlock-num-locks = 32
 hwlock-base-id = 32;
 #hwlock-cells = 1;
 };

 some-client {
 hwlocks = hwlock1 32, hwlock2 0;
 };

 The first args value is incorrect, and I expect the API to return an
 error. I don't think the API can make assumptions that all passed in
 values will be correct. What do you suggest that the API do here?

I think this is just one example of many potential mistakes the DT
developer can have, and that there's nothing really special about it.

What if the '5' digit below is a typo, and the actual hardware
assignment was supposed to be '4' ?

 some-client {
 hwlocks = hwlock1 5, hwlock2 5;
 };

I don't see how much different this mistake is from the one you
mentioned above. There's a limit to how much we can really catch DT
mistakes in the code, just like we couldn't really catch past mistakes
in the platform data structures.

If we can easily add some validations then great, but if we have to go
to great lengths just to gain very limited validations, then I'd vote
against doing so.

 One solution to handle #1 is to also make the hwlock-num-locks property
 also mandatory (even though it could have been read from a register

Yeah, this is awkward. We shouldn't impose this on implementations
like OMAP which don't need it.

Again I think for the very limited validation this buys us - it's not worth it.

 And, how do you propose we solve the problem of deferred probing?

It seems to me that hwspin_lock_request_specific failures should be
used by clients to defer their probing. Why wouldn't such a simple
solution work?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 1/5] Documentation: dt: add common bindings for hwspinlock

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna s-a...@ti.com wrote:
 This patch adds the generic common bindings used to represent
 a hwlock device and use/request locks in a device-tree build.

...

 Cc: Rob Herring robh...@kernel.org
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
  .../devicetree/bindings/hwlock/hwlock.txt  | 55 
 ++
  1 file changed, 55 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt

Please ask a DT maintainer to ACK this - otherwise I can't send this to Linus.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 2/5] Documentation: dt: add the omap hwspinlock bindings document

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna s-a...@ti.com wrote:
 HwSpinlock IP is present only on OMAP4 and other newer SoCs,
 which are all device-tree boot only. This patch adds the
 DT bindings information for OMAP hwspinlock module.

 Cc: Rob Herring robh...@kernel.org
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
  .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 24 
 ++

I need an ACK from a DT maintainer here as well, please try to get it.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna s-a...@ti.com wrote:
 +int of_hwspin_lock_get_id(struct device_node *np, int index)
 +{
 +   struct hwspinlock_device *bank;
 +   struct of_phandle_args args;
 +   int id;
 +   int ret;
 +
 +   ret = of_parse_phandle_with_args(np, hwlocks, #hwlock-cells, 
 index,
 +args);
 +   if (ret)
 +   return ret;
 +
 +   mutex_lock(hwspinlock_tree_lock);
 +   list_for_each_entry(bank, hwspinlock_devices, list)
 +   if (bank-dev-of_node == args.np)
 +   break;
 +   mutex_unlock(hwspinlock_tree_lock);
 +   if (bank-list == hwspinlock_devices) {
 +   ret = -EPROBE_DEFER;
 +   goto out;
 +   }

Is this the validation you mentioned which requires the existence of
hwspinlock/core: maintain a list of registered hwspinlock banks ?

I'm not convinced this is needed for several reasons:
- the user isn't using the lock yet at this point, and may only need
the id in order to communicate it to a remote processor
- if the user will try to use the lock prematurely,
hwspin_lock_request_specific should stop her
- moreover, hwspin_lock_request_specific must be the one who validates
the id, since in heterogeneous systems the user may get the id from a
remote processor and not via of_hwspin_lock_get_id

 hwspinlock/core: maintain a list of registered hwspinlock banks
adds complexity which must be very strongly justified.

If we're not sure there is a strong justification for it, we better
not merge it.

 +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id);
...
 +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_num_locks);

Do we really must expose these two helpers globally?

Can we instead make these static inline methods and embed them in
hwspinlock_internal.h ?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv6 5/5] hwspinlock/omap: add support for dt nodes

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna s-a...@ti.com wrote:
  static int omap_hwspinlock_probe(struct platform_device *pdev)
  {
 -   struct hwspinlock_pdata *pdata = pdev-dev.platform_data;
 +   struct device_node *node = pdev-dev.of_node;
 struct hwspinlock_device *bank;
 struct hwspinlock *hwlock;
 struct resource *res;
 void __iomem *io_base;
 -   int num_locks, i, ret;
 +   int num_locks, i, ret, base_id;

 -   if (!pdata)
 +   if (!node)
 return -ENODEV;

 +   ret = of_hwspin_lock_get_base_id(node);
 +   if (ret  0  ret != -EINVAL)
 +   return -ENODEV;
 +   base_id = (ret  0 ? ret : 0);

Does this mean you allow nodes not to have the base_id property? How
do we protect against multiple nodes not having a base_id property
then?

Implicitly assuming a base_id value (zero in this case) may not be always safe.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Nov 6, 2014 at 8:24 PM, Suman Anna s-a...@ti.com wrote:
 Ping on this. Can you review the latest series v6 [1] and pick it up for
 3.19? The MSM spinlock driver is also blocked/dependent on that series.

Sure, it's on my mind, I hope I'll get to it next week.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

On Fri, Sep 26, 2014 at 7:25 PM, Suman Anna s-a...@ti.com wrote:
 I am yet to receive any comments on v6, but that series should address
 both your need for a probe deferral and Ohad's request to not change any
 return types. Please give it a try and let me know if you have any comments.

Guys,

Just to let you know we have several holidays around here these days,
and I intend to review all pending patches immediately soon after.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Mon, Sep 15, 2014 at 10:39 PM, Suman Anna s-a...@ti.com wrote:
 These processors need to use their internal RAM for loading, which is
 not for generic usage by the kernel, so defining a CMA block for this
 memory doesn't make sense.

Ok - so just to make sure I understand, this is physical memory you
want to use, which belongs to the remote processor, and which isn't
mapped normally by the kernel?

 Will it suffice to replace the memcpy() with memcpy_toio()?

Yes, memcpy_toio should be fine (and then you don't need to cast the
cookie returned by ioremap).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 10/11] ARM: OMAP2+: AM33XX: Basic suspend resume support

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Sep 16, 2014 at 7:14 PM, Suman Anna s-a...@ti.com wrote:
 The current remoteproc infrastructure automatically calls rproc_boot
 only as part of the rpmsg/virtio stack (in remoteproc_virtio.c), but
 since the WkupM3 does not use rpmsg, there is no automatic booting of
 the WkupM3 processor.  This is the reason why rproc_boot is called as
 part of the WkupM3 driver probe sequence. What are your concerns here,
 and if you think this is not the right place do invoke rproc_boot, where
 do you expect it to be called?

The remoteproc layer is meant to hide hardware-specific details, and
allow high-level hardware-agnostic code to boot a remote processor, in
order to achieve some task, without even caring what kind of hardware
it is booting.

So generally we have some consumer driver asking remoteproc to boot a
remote processor, and in turn, remoteproc asking a hardware-specific
vendor driver to take care of the hardware details like actually
taking the remote processor out of reset.

The consumer driver is some code that deals with some hardware
agnostic task. Today the only consumer we have is rpmsg, so it's
perfectly reasonable if remoteproc needs to be adapted a bit to
accommodate other consumers as they show up.

Can you think of any component in your code that is not necessarily
hardware specific, and that one day might be useful for other vendors?
Can you describe the task you're trying to achieve, the entities
involved and the flow between them?

 Also do note that, there is no way
 at present to retrieve the struct rproc for a given remote processor, to
 be able to invoke the rproc_boot from a different layer.

It's perfectly ok to make struct rproc public if we have a consumer
that requires it.

 Splitting this would still require some kind of notifier from remoteproc
 for the other layers for them to know that the WkupM3 remote processor
 has been loaded and booted successfully. This is also done as part of
 the WkupM3 driver at the moment.

Are there entities, other than the one that calls rproc_boot, that
needs to know that the WkupM3 is up? if so, let's discuss who should
notify them - remoteproc or the actual invoker of rproc_boot. It
probably depends on who those entities are and what's their relation,
if any, to the WkupM3 driver.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 10/11] ARM: OMAP2+: AM33XX: Basic suspend resume support

[Ohad Ben-Cohen]

On Wed, Sep 10, 2014 at 12:10 AM, Kevin Hilman khil...@kernel.org wrote:
 What I think you need to do (and what I've recommended at least once in
 earlier reviews) put all the (non-rproc) wkup_m3 IPC into into one
 driver and create a well-described, well-documented API that the
 platform PM code will use.

 IMO, the current split is very difficult to read/understand, which
 means it will even more difficult to maintain.

I strongly agree.

A remoteproc driver should generally only register its
hardware-specific implementation of the rproc_ops via the remoteproc
framework. It is not expected to expose public API of its own - that's
why we have the generic remoteproc layer for. It makes perfect sense
not to use rpmsg for communications if it's not lightweight enough,
but exposing a new set of IPC API should take place in a separate
well-documented driver.

In addition, the suggested remoteproc driver seems to act both as a
low-level hardware-specific driver that plugs into remoteproc (by
registering rproc_ops via the remoteproc framework), and also as a
high-level driver that utilizes the remoteproc public API (by calling
rproc_boot). This alone calls for scrutinizing the design as this is
not very intuitive.

At least for the remoteproc part: if you could provide a simple and
straight-forward remoteproc driver that just implements the rproc_ops,
this could be merged very quickly. The rest of the code most probably
belongs in a different entity, just like Kevin suggested.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 10/11] ARM: OMAP2+: AM33XX: Basic suspend resume support

[Ohad Ben-Cohen]

On Tue, Sep 9, 2014 at 1:30 AM, Kevin Hilman khil...@linaro.org wrote:
 To me, it's not terribly clear how you made the split between this PM
 core code an the remoteproc code.  In the changelog for the remoteproc
 patch, it states it's to load the firmware for and boot the wkup_m3.
 But, while parts of the IPC are here in pm33xx.c, parts of the IPC are
 also inside the remoteproc driver, so I'm quite curious if that's OK
 with the remoteproc maintainers.  Either way, please make it clearer how
 and why you made the split, and please isolate the wkup_m3 IPC/protocol
 from this code.  Think of people wanting to rework/extend the wkup_m3
 firmware.  They shouldn't be messing around in here, but rather inside a
 driver specificaly for the wkup_m3.

I haven't looked at the code very thoroughly yet, but generally a
remoteproc driver should only implement the three start/stop/kick
rproc_ops, and then register them via the remoteproc framework.
Exposing additional API directly from that driver isn't something we
immediately want to accept.

If relevant, we would generally prefer to extend remoteproc instead,
so other platform-specific drivers could utilize that functionality as
well. Or rpmsg - if we're missing some IPC functionality.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Jul 29, 2014 at 10:33 PM, Suman Anna s-a...@ti.com wrote:
 We currently have two usecases. The primary usecase is the WkupM3
 processor on TI Sitara AM335x/AM437x SoCs used for suspend/resume
 management. This series is a dependency for the WkupM3 remoteproc driver
 that Dave posted [1]. More details are in section 8.1.4.6 of the AM335x
 TRM [2]. The program/data sections for this processor all _needs_ to be
 in the two internal memory RAMS (16K Unified RAM and 8K Data RAM), and
 there is no MMU for this processor. The current RSC_CARVEOUT and
 RSC_DEVMEM do not fit to describe this type of memory (we neither
 allocate memory through dma api nor do we need to map these into an MMU).

Thanks for the details.

Can we define a CMA block for these regions, and then just use
carveout resource entries instead of the ioremap approach?

This may require some changes in remoteproc which we'll need to think
about, but it sounds like it may fit the problem better instead of
forcing ioremap to provide a regular pointer (we're supposed to use
ioremaped memory only with memory primitives such as readl/writel/..).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rpmsg: compute number of buffers to allocate from vrings

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Aug 12, 2014 at 7:19 PM, Suman Anna s-a...@ti.com wrote:
 Yes, I was playing around with using less buffers in the remoteproc
 resource table for the vrings. The remoteproc virtio code creates the
 vrings using the number of buffers based on .num field value of struct
 fw_rsc_vdev_vring in the resource table. The virtio rpmsg probe code
 though tries to set up the receive buffers for the same virtqueue based
 on the current hard-coded value of 512 buffers and virtqueue_add_inbuf
 would fail as the virtqueue is created with less number of buffers and
 throws a WARN_ON.

Gotcha - thanks for the details.

Limiting the number of buffers in case the vrings are too small makes
sense, but let's use RPMSG_NUM_BUFS as an upper bound, so wacky
resource tables won't trigger unreasonable memory waste.

Something in the lines of:

vrp-num_bufs = min(PMSG_NUM_BUFS, virtqueue_get_vring_size(vrp-rvq) * 2);

Should probably do the trick.

Does this satisfy your requirement?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] hwspinlock for 3.17

[Ohad Ben-Cohen]

The following changes since commit 9a3c4145af32125c5ee39c0272662b47307a8323:

  Linux 3.16-rc6 (2014-07-20 21:04:16 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/hwspinlock.git
tags/hwspinlock-3.17

for you to fetch changes up to ceca89e89ee21a3af050c3ea4c634b3be0e34d51:

  hwspinlock: enable OMAP build for AM33xx, AM43xx  DRA7xx
(2014-07-29 11:46:28 +0300)


Two small hwspinlock changes for better OMAP support, coming
from Suman Anna.


Suman Anna (2):
  hwspinlock/omap: enable module before reading SYSSTATUS register
  hwspinlock: enable OMAP build for AM33xx, AM43xx  DRA7xx

 drivers/hwspinlock/Kconfig   |  2 +-
 drivers/hwspinlock/omap_hwspinlock.c | 27 ---
 2 files changed, 21 insertions(+), 8 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rpmsg: compute number of buffers to allocate from vrings

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 11:53 PM, Suman Anna s-a...@ti.com wrote:
 The buffers to be used for communication are allocated during
 the rpmsg virtio driver's probe, and the number of buffers is
 currently hard-coded to 512. Remove this hard-coded value, as
 this can vary from one platform to another or between different
 remote processors. Instead, rely on the number of buffers the
 virtqueue vring is setup with in the first place.

Is there a specific problem you bumped into which you are fixing with
this approach? Can you please describe it?

I'm concerned that coupling the vring size with coherent memory
allocated by rpmsg may not be safe. It'd also be an implicit side
effect that may surprise users, so let's consider our alternatives.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

On Mon, Aug 4, 2014 at 6:48 PM, Suman Anna s-a...@ti.com wrote:
 On 08/04/2014 06:50 AM, Ohad Ben-Cohen wrote:
 On Tue, Jul 29, 2014 at 7:10 PM, Suman Anna s-a...@ti.com wrote:
 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.

 That's perfect, thanks. Can you please add this use case description
 to the commit log?

 I kept the current description generic, but sure, I can add this
 specific usecase examples to the commit log. I will post the revised
 patches once rc1 is out.

Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

On Tue, Jul 29, 2014 at 7:10 PM, Suman Anna s-a...@ti.com wrote:
 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.

That's perfect, thanks. Can you please add this use case description
to the commit log?

This way we'll be able to recover it easily one day if needed.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Jul 8, 2014 at 7:22 PM, Suman Anna s-a...@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?

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.

 Cc: Sjur Brændeland sjur.brandel...@stericsson.com

Sjur is no longer with STE, so no point in cc'ing his old email address.

 +   /*
 +* 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.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Jul 8, 2014 at 7:22 PM, Suman Anna s-a...@ti.com wrote:
 A remote processor may need to load certain firmware sections into
 internal memories (eg: RAM at L1 or L2 levels) for performance or
 other reasons.

Can you please provide as much details as you can about the scenario
you need this for? what hardware, what sections, which specific
memory, what's the use case, numbers, sizes, everything.

I'd like to better understand the use case please.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

On Thu, Jul 24, 2014 at 6:13 PM, Suman Anna s-a...@ti.com wrote:
 Is there a specific reason for using the put_sync variant here? If
 not, let's just use the regular put().

 There was no particular reason, you can change it.

Thanks for confirming. I've applied both patches to remoteproc's
for-next branch.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 2:00 AM, Suman Anna s-a...@ti.com wrote:
 The number of hwspinlocks are determined based on the value read
 from the IP block's SYSSTATUS register. However, the module may
 not be enabled and clocked, and the read may result in a bus error.

 This particular issue is seen rather easily on AM33XX, since the
 module wakeup is software controlled, and it is disabled out of
 reset. Make sure the module is enabled and clocked before reading
 the SYSSTATUS register.

 Signed-off-by: Suman Anna s-a...@ti.com
...
 +   /*
 +* runtime PM will make sure the clock of this module is
 +* enabled again iff at least one lock is requested
 +*/
 +   ret = pm_runtime_put_sync(pdev-dev);

Is there a specific reason for using the put_sync variant here? If
not, let's just use the regular put().

Let me know, and I can do the change while applying these two patches.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 03/15] hwspinlock/core: maintain a list of registered hwspinlock banks

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 12:14 AM, Suman Anna s-a...@ti.com wrote:
 I'm not sure we need this patch.

 This patch is needed if we use the controller-phandle + args specifier
 for requesting hwlocks by a client, as we need to translate
 controller-phandle to the corresponding hwspinlock_device.

 Looks like we still don't have a closure on the semantics of how
 clients have to request a lock in DT. You are suggesting something like
 hwlocks = global_lock1 global_lock2 ...;

 whereas this patch is built to support based on comments from
 DT-maintainers,
 hwlocks = controller-phandle lock-specifier1, controller-phandle
 lock-specifier2...;

I'm actually ok with this suggestion and haven't suggested otherwise.

All I propose is that we add the base_id property to the controller
node (as you have done in the subsequent patches), and then drivers
will be able to infer the global lock id from the DT data by adding
the controller's base_id to the lock specifier.

Controllers with non standard lock indexing may use an xlate() method
if needed but frankly this is fictional right now. We can start
without this, and add it later when needed, as this doesn't affect the
DT data.

With the global lock id in hand, drivers could simply use the existing
hwspin_lock_request_specific API to obtain a specific lock, and then
we don't need this patch.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 04/15] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 12:14 AM, Suman Anna s-a...@ti.com wrote:
 Do we have a use case today that require the xlate() method?

 If not, let's remove it as we could always add it back if some new
 hardware shows up that needs it.

 The xlate() method is to support the phandle + args specifier way of
 requesting hwlocks, platform implementations are free to implement their
 own xlate functions, but the above supports the simplest case of
 controller + relative lock index within controller.

Do we have a use case for a different implementation other than the
simplest case? If not, it seems to me this will just become redundant
boilerplate code (every platform will use the simple xlate method).

 This function again is to support the phandle + args specifier way of
 requesting hwlocks, the hwspin_lock_request_specific() is invoked
 internally within this function, so we are still reusing the actual
 request code other than handling the DT parsing portion. If we go back
 to using global locks in client hwlocks property, we don't need a
 of_hwspin_lock_get_id(), the same can be achieved using the existing DT
 function, of_property_read_u32_index().

I think you may have misunderstood me, sorry. I'm ok with the phandle
+ args specifier. I just think we can use it, together with the
base_id property, to infer the global lock id from the DT data. This
is not only a must to support heterogenous multi-processing systems,
it will also substantially simplify the code.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 05/15] hwspinlock/omap: add support for dt nodes

[Ohad Ben-Cohen]

On Wed, Jul 2, 2014 at 10:42 PM, Suman Anna s-a...@ti.com wrote:
 Yeah, I did this since we only had 1 instance, and used the same value
 as used in the non-DT legacy code. Once I fold back Patch 8 that adds
 the hwlock-base-id property, this will be assigned by reading that property.

Sounds good, thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 04/15] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 8:35 PM, Suman Anna s-a...@ti.com wrote:
 Not at the moment, with the existing platform implementations. So, if I
 understand you correctly, you are asking to leave out the xlate ops and
 make the of_hwspin_lock_simple_xlate() internal until a need for an
 xlate method arises.

Yes, please. It seems to me this way we're reducing complexity without
compromising anything.

 This also means, we only support a value of 1 for #hwlock-cells.

When a different #hwlock-cells value shows up, someone would have to
write a new xlate implementation anyway. At the same time, the xlate
ops could be brought back quite easily.

Since we're not imposing anything on the DT data, this doesn't affect
our ability to support these scenarios in the future, but unless I'm
missing a current use case, these scenarios right now seems somewhat
fictional.

 But, that would mean DT-based client users would have to invoke two
 function calls to request a hwspinlock. We already have an API to get
 the lock id given a hwspinlock - hwspin_lock_get_id(), so I added the OF
 API for requesting a lock directly rather than giving an OF API for
 getting the lock id. This is in keeping in convention with what other
 drivers do normally - a regular get and an OF get. I can modify it if
 you still prefer the OF API for getting a global lock id, but I feel its
 a burden for client users.

Let's discuss this.

I was actually thinking of the more general use case of an heterogenous system:

- driver gets the global lock id from a remote processor
- driver then requests the specific lock

Looking at these cases, the OF scenario only differs in the small part
of fetching the lock id, and if we keep the regular request_specific
API we'll have more common code between drivers.

What do you think?

 Also, do you prefer an open property-name in client users (like I am
 doing at the moment) or imposing a standard property name hwlocks?

Good point - I think we can start with an imposed hwlocks name, and
evolve in the future as needed (again, only because we're not really
imposing anything on the DT data - this is just kernel code that we
could always extend as needed).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 03/15] hwspinlock/core: maintain a list of registered hwspinlock banks

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 8:28 PM, Suman Anna s-a...@ti.com wrote:
 OK, but we would still require this function to lookup the registered
 device from the controller-phandle to retrieve the base_id.

Can we retrieve the base_id from the parent DT node itself?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 03/15] hwspinlock/core: maintain a list of registered hwspinlock banks

[Ohad Ben-Cohen]

Hi Suman,

On Thu, May 1, 2014 at 3:34 AM, Suman Anna s-a...@ti.com wrote:

 The hwspinlock_device structure is used for registering a bank of
 locks with the driver core. The structure already contains the
 necessary members to identify the bank of locks. The core does not
 maintain the hwspinlock_devices itself, but maintains only a radix
 tree for all the registered locks. A specific lock can be requested
 by users using a global lock id, and any device-specific fields
 can be retrieved through a reference to the hwspinlock_device in
 each lock.

 The global lock id, however, is not friendly to be requested for
 users using the device-tree model. The device-tree representation
 will typically have each of the hwspinlock devices represented as
 a DT node, and a specific lock can be requested using the device's
 phandle and a lock specifier. Add support to the core therefore to
 maintain all the registered hwspinlock_devices, so that a device
 can be looked up and a specific lock belonging to the device
 requested through a phandle + args approach.

 Signed-off-by: Suman Anna s-a...@ti.com

I'm not sure we need this patch.

It seems to me that the global lock id can be the base_id + lock
index, where the former should be a property of the parent dt node,
and the latter can just be the phandle argument. Then, with the global
lock id in hand, drivers could just use the existing
hwspin_lock_request_specific API.

If future hardware will bring a more complex scenario, we could then
introduce the xlate proposal to resolve it. As long as we're not
changing the dt data, and this is all handled by kernel code, then I'd
prefer opting for less code now as long as it addresses the
requirements.

Please let me know if currently there is a use case that can't be
addressed using this simpler model.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 04/15] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Thu, May 1, 2014 at 3:34 AM, Suman Anna s-a...@ti.com wrote:
 2. The of_hwspin_lock_simple_xlate() is a simple default
translator function for hwspinlock provider implementations
that use a single cell number for requesting a specific lock
(relatively indexed) within a hwlock bank.

Do we have a use case today that require the xlate() method?

If not, let's remove it as we could always add it back if some new
hardware shows up that needs it.

As long as the dt data is unchanged, we should generally only add
kernel code that we really need.

 3. The of_hwspin_lock_request_specific() API can be used by
hwspinlock clients to request a specific lock using the
phandle + args specifier. This function relies on the
implementation providing back a relative hwlock id within
the bank from the args specifier.

It seems to me we can just introduce a of_hwspin_lock_get_id() method
which will provide the global lock id to dt users, with which they
could just invoke the existing hwspin_lock_request_specific(). This
way we will have more common code between dt users and users who get
the lock id from a remote processor.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 05/15] hwspinlock/omap: add support for dt nodes

[Ohad Ben-Cohen]

Hi Suman,

On Thu, May 1, 2014 at 3:34 AM, Suman Anna s-a...@ti.com wrote:
  static int omap_hwspinlock_probe(struct platform_device *pdev)
  {
 -   struct hwspinlock_pdata *pdata = pdev-dev.platform_data;
 +   struct device_node *node = pdev-dev.of_node;
 struct hwspinlock_device *bank;
 struct hwspinlock *hwlock;
 struct resource *res;
 void __iomem *io_base;
 int num_locks, i, ret;
 +   int base_id = 0;

We shouldn't implicitly assume base_id is zero: let's explicitly
protect against potential subsequent invocations of
omap_hwspinlock_probe.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 06/15] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

Hi Suman,

On Thu, May 1, 2014 at 3:34 AM, Suman Anna s-a...@ti.com wrote:
 The number of hwspinlocks are determined based on the value read
 from the IP block's SYSSTATUS register. However, the module may
 not be enabled and clocked, and the read may result in a bus error.

 This particular issue is seen rather easily on AM33XX, since the
 module wakeup is software controlled, and it is disabled out of
 reset. Make sure the module is enabled and clocked before reading
 the SYSSTATUS register.

This seems like a valid fix that is independent of this patch series.

Feel free to submit it separately if you want, so we can get it merged.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv5 07/15] hwspinlock/omap: enable build for AM33xx, AM43xx DRA7xx

[Ohad Ben-Cohen]

Hi Suman,

On Thu, May 1, 2014 at 3:34 AM, Suman Anna s-a...@ti.com wrote:
 HwSpinlocks are supported on AM33xx, AM43xx and DRA7xx SoC
 device families as well. The IPs are identical to that of
 OMAP4/OMAP5, except for the number of locks.

 Add a depends on to the above family of SoCs to enable the
 build support for OMAP hwspinlock driver for any of the above
 SoC configs.

 Signed-off-by: Suman Anna s-a...@ti.com

This looks too like an independent change. If you want it merged
without waiting for the entire series please submit it separately.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/7] omap hwspinlock dt support

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Mar 18, 2014 at 1:46 AM, Suman Anna s-a...@ti.com wrote:
 So far, we have not come across multiple controllers. I see your point,
 and I think this also depends on the semantics of how you exchange the
 lock id number. The agreement at the moment is on base_ids across
 multiple SoC components. If the semantics involve exchanging the
 controller instance, for example, then we might get away with it. But
 that probably involves adding additional helpers to retrieve controller
 instance in addition to lock number, or some other similar functions.

Yes, this could be done too, but I agree it is less simple with no real win.

 Sorry, I should have rephrased it better - by order, I meant the
 inherent order between board early code and other drivers. With DT, we
 cannot guarantee that right, as specific locks are requested from drivers.

Yeah.

 Understood. And we may have to assign the client association with a lock
 as well. These are core changes that were actually not needed in the
 non-DT case due to the inherent order as stated above. So, are you
 suggesting that we add one more property to the controller node to mark
 which are reserved, or rely on constructing this through DT tree parsing?

I guess this is a question to the DT folks; both approaches work from
hwspinlock perspective.

In the past Arnd Benoit and myself were happy with adding one more
property to the controller node, but this might be somewhat error
prone as it leaves room for mistakes - developers can add hwlock
phandles and forget to update the reserved property in the controller
node.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/7] omap hwspinlock dt support

[Ohad Ben-Cohen]

Hi Suman,

On Sat, Mar 15, 2014 at 1:58 AM, Suman Anna s-a...@ti.com wrote:
 The series doesn't change the semantics of hwspinlock registration or adds a
 new OF controller registration function. Implementations would still need to
 register a controller using a base_id and number of locks. The series rather
 adds a DT-friendly function _ONLY_ for requesting a specific hwlock, and
 there are no restrictions on the args specifier being relative id numbers.
 Though this is what the simple default xlate helper does (most common
 usage), the added xlate ops and #hwlock-cells should allow individual
 implementation drivers to adjust any variations, and return a relative lock
 w.r.t its registered base_id, as this is how a lock gets registered in the
 first place.

I might be missing something, but why can't we have the
specifier+base_id be the hwlock id and then we can entirely drop most
of the core changes in this patch-set? I realize we couldn't easily
support sparse id numbers, but not sure this is relevant to
hwspinlocks? do we have a use case that couldn't be supported in this
case?

 I actually started out this series with the base_id property, and dropped it
 in v3 based on comments looking at it from the request-specific-lock
 semantics with DT. That said, the drivers still need to manage a 'base_id'
 needed for registration when they get probed for multiple controllers.
 Getting the base_id from DT _may_ be useful just for registration purposes,
 but for requesting a hwlock, a controller phandle and an implementation
 defined args-specifier should suffice IMHO.

How could drivers know what the base_id is if DT doesn't provide it?
please note that we can't depend on order of controller probing; the
hwlock id numbers cannot depend on implementation details.

 The exact notion of informing the hwspinlock core about a list of reserved
 locks is missing at the moment (even in the non-DT case). I am not sure if
 this got lost during the conversion of the registration from per lock to
 registering a bank of locks together, or if it is implied by the base_id +
 num_locks combination. The core today supports requesting only those locks
 that were actually registered, whether allocating a free one dynamically or
 giving a specific one.

Before DT came along, early board code could have reserved specific
hwspinlocks if needed. Now with DT, we should add the list of reserved
locks to the controller node, in order to prevent them from being
dynamically allocated by others.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/7] omap hwspinlock dt support

[Ohad Ben-Cohen]

Hi Suman,

On Mon, Mar 17, 2014 at 9:10 PM, Suman Anna s-a...@ti.com wrote:
 base_id would be a property (if added) of the hwspinlock controller node,
 and from DT perspective, we will be using the phandle for the controller
 anyway. So, using a base_id+specifier seems redundant, as the specifier is
 already w.r.t a hwspinlock controller node.  It is best to leave the base_id
 out, just use the specifier. This is pretty much the standard practice
 (GPIOs, DMAs, etc all follow this).

Do you mean hwspin_lock_get_id() will return just the specifier
instead of base_id+specifier? This is problematic, because Linux will
not be able to communicate this lock id outside to a different core
running a different OS: that OS will have no idea what hwlock
controller this is relative to.

 Please see the comments from Mark regarding the same on an earlier version.

I think I understand and agree with Mark generally, but in this case,
the hwlock id is not an implementation detail. Unlike GPIOs/DMAs, this
id number is global and system-wide (Linux is just one component in
the system, and must use the same predefined id numbers all other
cores use, otherwise it will be impossible to use those hwlocks for
multi-core synchronization).

 Yes, I agree this is an issue if we have to have the base_ids fixed per
 controller.

Do you see any other way this could work if the base_ids were not
predefined? Linux and some other core running a different OS must
agree on the id numbers of the hardware locks we have in the system.

 Before DT came along, early board code could have reserved specific
 hwspinlocks if needed. Now with DT, we should add the list of reserved
 locks to the controller node, in order to prevent them from being
 dynamically allocated by others.


 But that strictly relied on the order of requests without any core changes
 in the hwspinlock core, right.

I don't think so, you could request a specific lock by id number.

 With DT, the early board code is much simplified. Looking at the same
 scenario from DT case, it seems kinda redundant to specify a set of reserved
 locks both in the controller node, as well as the respective client drivers,
 as there is almost no platform data with DT. The only use case for DT client
 nodes would be for requesting specific locks. I agree with the problem you
 described, and I think it will require a different set of changes to the
 core.

Exactly. The platform-specific hwspinlock driver will have to inform
the core, upon registration, which of the locks are already reserved.
In turn, the core will never provide them to clients that dynamically
allocate an hwlock: they will be provided only to clients that ask for
them specifically (using phandle+specifier).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

On Fri, Mar 14, 2014 at 5:23 PM, Josh Cartwright jo...@codeaurora.org wrote:
 So, are you suggesting that because fatal errors should be extremely
 rare, a consuming driver should just assume that if NULL is returned
 from a hwspin_lock_request*() function that it was the device not yet
 probed case that was hit?

No - it's not the scarcity, it's the severity.

The error path that will be optimized here is an invalid id. If this
happens, the consumer will crash and burn, and I'm not sure that
slightly optimizing his death is very interesting?

BTW the hwspinlock core once did use ERR_PTR and friends, and it was
changed due to convincing arguments against that methodology on this
mailing list. We can change it back but we need a strong(er) case.

 Note that having the consumer/hwspinlock device relationship modeled in
 devicetree introduces more potential failure cases...

Yeah. Even the error above, presumed to be EPROBE_DEFER, may be a
symptom of some other fatal error that occurred, and we can't be sure
that a future request will surely be satisfied. So before we bloat our
code, I suggest that we wait for consumers to show up and see if
there's real benefit.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Mar 4, 2014 at 7:38 PM, Suman Anna s-a...@ti.com wrote:
 Do you have any objections to the return code convention change?

 Unless strictly needed, I prefer we don't switch to the ERR_PTR code
 convention, as it reduces code readability and increases chances of
 user bugs.

 In our case, switching to ERR_PTR and friends seems only to optimize a
 few error paths, and I'm not sure it's a big win over simplicity.


 When introducing the ability to reference a hwspin lock via a phandle
 in device tree it makes a big difference to be able to differ between
 the case of initialization failed or device not yet probed; so
 that the client knows if it should fail or retry later.


 Can you confirm the changes you want me to make, so that I can refresh and
 post a v5 for 3.15?

Sorry, I missed your replies for some reason.

I prefer we stick with the current error handling code because I find
the alternative inferior (as long as it's not strictly needed).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

On Sun, Mar 2, 2014 at 10:19 PM, Bjorn Andersson bj...@kryo.se wrote:
 When introducing the ability to reference a hwspin lock via a phandle
 in device tree it makes a big difference to be able to differ between
 the case of initialization failed or device not yet probed; so
 that the client knows if it should fail or retry later.

I'm not convinced.

The only advantage this brings is to avoid retrying in case a fatal
error has occurred. Such fatal errors are extremely rare, and when
they show up - extremely painful, and I suspect that optimizing them
wouldn't be a big win.

OTOH, keeping the code easier to read and less error prone is a big
win. I prefer we keep it simple for now.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/7] omap hwspinlock dt support

[Ohad Ben-Cohen]

Hi Suman, Mark,

On Mon, Feb 24, 2014 at 8:14 PM, Suman Anna s-a...@ti.com wrote:
 Mark, Ohad,
...
 Gentle reminder, can you provide your acks/comments?

Sorry for the late jump in.

I have a few comments:

- Hardware spinlocks must have global and system-wide id numbers;
these numbers cannot be maintained internally by the Linux Kernel.
Think of an SoC with several asynchronous heterogeneous processors,
each of which is running a different OS, and they all need to use a
specific hardware spinlock in order to share some resource. For that
to happen, every hwlock must have a predefined and deterministic id
number which is global in the system. We can't have those id numbers
be relative to an hwlock controller, and we can't have two hwlock
controllers share the same id numbers.

- I suspect the simplest and most straight forward way to achieve this
is by (a) bringing back the concept of the base_id property, and (b)
letting the global hwlock id be the DT identifier (plus the base_id)
and then providing it directly to the drivers when needed. The latter
is required in order to support dynamically allocation of hwlocks, in
which case Linux must know the global system-wide id number, and then
share it with the other asynchronous OSes via some IPC.

- If we feel there's no way any system is going to have more than a
single hwlock controller, then we can live without a base_id property,
but then this needs to be clearly documented and prohibited. Today
both the hwlock DT representation, and the coupled kernel code,
implicitly allow this anomaly to exist.

- Hwlock controller nodes should have a list of reserved locks (those
locks for which other nodes have a phandle+identifier pointing at) to
prevent those locks from being dynamically allocated by eager drivers.

Most of these issues were discussed Arnd, Benoit and myself back then,
please see below:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-September/064265.html

Let's discuss,

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 4/7] hwspinlock/core: add common OF helpers

[Ohad Ben-Cohen]

On Mon, Feb 10, 2014 at 9:14 PM, Suman Anna s-a...@ti.com wrote:
 On 02/07/2014 04:49 PM, Bjorn Andersson wrote:
 It seems to be standard practice to pass the error value back to the
 consumer, so you should
 return ERR_PTR(ret); here instead of the NULL...


 I have modelled the return values in this function based on the return
 values in the existing hwspin_lock_request interfaces. I would need to
 change those functions as well.

 Ohad,
 Do you have any objections to the return code convention change?

Unless strictly needed, I prefer we don't switch to the ERR_PTR code
convention, as it reduces code readability and increases chances of
user bugs.

In our case, switching to ERR_PTR and friends seems only to optimize a
few error paths, and I'm not sure it's a big win over simplicity.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] remoteproc fixes

[Ohad Ben-Cohen]

With the lists this time

On Wed, Jul 10, 2013 at 6:17 PM, Ohad Ben-Cohen o...@wizery.com wrote:
 The following changes since commit 9e895ace5d82df8929b16f58e9f515f6d54ab82d:

   Linux 3.10-rc7 (2013-06-22 09:47:31 -1000)

 are available in the git repository at:

   git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
 tags/remoteproc-3.11-fixes

 for you to fetch changes up to 95cee62cb4776a65229a6b6d5969be56589d95c1:

   remoteproc: Cocci spatch memdup.spatch (2013-07-01 17:23:58 +0300)

 
 Trivial remoteproc fixes by Suman Anna, Wei Yongjun and Thomas Meyer.

 
 Suman Anna (3):
   remoteproc: fix checkpatch errors in remoteproc code
   remoteproc/omap: fix a sparse warning
   remoteproc: free carveout memories only after unmapping them

 Thomas Meyer (1):
   remoteproc: Cocci spatch memdup.spatch

 Wei Yongjun (1):
   remoteproc: fix error return code in rproc_fw_boot()

  drivers/remoteproc/remoteproc_core.c  | 24 
  drivers/remoteproc/remoteproc_debugfs.c   |  3 +--
  drivers/remoteproc/remoteproc_internal.h  |  4 ++--
  include/linux/platform_data/remoteproc-omap.h |  2 +-
  4 files changed, 16 insertions(+), 17 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: find real users and drivers of rpmsg

[Ohad Ben-Cohen]

Hi,

On Mon, Jul 8, 2013 at 10:31 AM, Barry Song 21cn...@gmail.com wrote:
 hi Ohad/all,
 i am trying to find some real users of rpmsg, here i only get
 samples/rpmsg/rpmsg_client_sample.c, does it mean other real drivers
 are out of mainline?

Yes

 where could i get them?

TI maintains them in internal trees, some of which might be public.
I'm looping in Suman from TI who might be able to refer you to some

 i am also trying to find source codes running in Cortex-M3 which use
 rpmsg and hope to find some Linux userspace codes which use rpmsg too.
 Thanks
 barry
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] hwspinlock for 3.10

[Ohad Ben-Cohen]

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/hwspinlock.git
tags/hwspinlock-3.10

for you to fetch changes up to 8ae053d62e88c400330ebaf27558bf02dde5a1fa:

  hwspinlock/omap: support OMAP5 as well (2013-04-05 09:11:17 +0300)


A single patch from Vincent extending OMAP's hwspinlock support to OMAP5.


Vincent Stehlé (1):
  hwspinlock/omap: support OMAP5 as well

 drivers/hwspinlock/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] rpmsg for 3.10

[Ohad Ben-Cohen]

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg.git tags/rpmsg-3.10

for you to fetch changes up to 397944df3290ddc46dcc6a08cd71fb560700431b:

  rpmsg: fix kconfig dependencies for VIRTIO (2013-04-21 16:32:29 +0300)


A small pull request consisting of:
- Make rpmsg process all pending messages instead of just
  one, from Robert Tivy
- Fix Kconfig dependency on VIRTUALIZATION, from Suman.
  Note: this was submitted late during the 3.9 rc cycle and it
  seemed appropriate to wait with it for the merge window.
- Belated addition of an rpmsg entry to the MAINTAINERS file.
  People seem to look for this.


Ohad Ben-Cohen (1):
  MAINTAINERS: add rpmsg entry

Robert Tivy (1):
  rpmsg: process _all_ pending messages in rpmsg_recv_done

Suman Anna (1):
  rpmsg: fix kconfig dependencies for VIRTIO

 MAINTAINERS  |  8 +++
 drivers/rpmsg/Kconfig|  1 +
 drivers/rpmsg/virtio_rpmsg_bus.c | 49 
 3 files changed, 44 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] remoteproc for 3.10

[Ohad Ben-Cohen]

There's a trivial merge conflict with a Kconfig typo that got fixed
during the rc cycle,
but otherwise:

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/remoteproc-3.10

for you to fetch changes up to b9777859ec015a78dae1476e317d04f851bfdd0d:

  remoteproc: fix kconfig dependencies for VIRTIO (2013-04-21 16:30:22 +0300)


This pull request contains:
- Some refactoring, cleanups and small improvements
  from Sjur Brændeland. The improvements are mainly
  about better supporting varios virtio properties
  (such as virtio's config space, status and features).
  I now see that I messed up while commiting one of Sjur's
  patches and erroneously put myself as the author, as well
  as letting a nasty typo sneak in. I will not fix this in
  order to avoid rebasing the patches. Sjur - sorry!
- A new remoteproc driver for OMAP-L13x (technically a
  DaVinci platform) from Robert Tivy.
- Extend OMAP support to OMAP5 as well, from Vincent Stehlé.
- Fix Kconfig VIRTUALIZATION dependency, from Suman Anna
  (a non-critical fix which arrived late during the rc cycle).


Ohad Ben-Cohen (1):
  remoteproc: perserve resource table data

Robert Tivy (2):
  remoteproc: support default firmware name in rproc_alloc()
  remoteproc/davinci: add a remoteproc driver for OMAP-L13x DSP

Sjur Brændeland (7):
  remoteproc: refactor rproc_elf_find_rsc_table()
  remoteproc: add find_loaded_rsc_table firmware ops
  remoteproc: parse STE-firmware and find resource table address
  remoteproc: code cleanup of resource parsing
  remoteproc: calculate max_notifyid by counting vrings
  remoteproc: support virtio config space.
  remoteproc: set vring addresses in resource table

Suman Anna (1):
  remoteproc: fix kconfig dependencies for VIRTIO

Vincent Stehlé (1):
  remoteproc/omap: support OMAP5 too

 drivers/remoteproc/Kconfig |  27 ++-
 drivers/remoteproc/Makefile|   1 +
 drivers/remoteproc/da8xx_remoteproc.c  | 324 +
 drivers/remoteproc/remoteproc_core.c   | 211 ---
 drivers/remoteproc/remoteproc_elf_loader.c | 100 ++---
 drivers/remoteproc/remoteproc_internal.h   |  15 +-
 drivers/remoteproc/remoteproc_virtio.c |  79 +--
 drivers/remoteproc/ste_modem_rproc.c   |  45 ++--
 include/linux/remoteproc.h |  13 +-
 9 files changed, 677 insertions(+), 138 deletions(-)
 create mode 100644 drivers/remoteproc/da8xx_remoteproc.c
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: adding rpmsg.git to linux next

[Ohad Ben-Cohen]

Hi Stephen,

On Tue, Apr 16, 2013 at 3:07 AM, Stephen Rothwell s...@canb.auug.org.au wrote:
 On Mon, 15 Apr 2013 09:28:17 +0300 Ohad Ben-Cohen o...@wizery.com wrote:
 Could you please add:

 git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg.git#for-next

 to linux-next to include new stuff coming from Rob?

 Well, you could tell me something about it.  Like what is in it and how
 it will be merged up to Linus.  Does this have anything to do with the
 remoteproc tree I already include?  Who is the official maintainer (there
 is no mention of drivers/rpmsg in the MAINTAINERS file).

Sorry for not mentioning this.

This tree isn't new and is being used to send pull requests to Linus
for some time. I guess that most (all?) of the pull requests contained
fixes which needed no linux-next presence, so it never occurred to me
the tree isn't part of linux-next.

Some background:

Rpmsg is a virtio-based messaging bus that allows kernel drivers to communicate
with entities running on remote processors available on the system. In
turn, drivers could then
expose appropriate user space interfaces, if needed.

Rpmsg is essentially a multiplexer providing communication channels
which rpmsg drivers utilizes; it does not handle the physical state of
the remote processor, and in fact is completely decoupled from
remoteproc, which does focus on the physical state of the remote
processor (but has no communications aspect). For much more
information, please see Documentation/rpmsg.txt.

I maintain rpmsg (by sending pull requests to Linus), and for some
reason never added a MAINTAINERS entry for it. I can fix that quite
easily, let me know if this helps:

commit d8115db52b99eefb99bcbf992edc349e691b4ca7
Author: Ohad Ben-Cohen o...@wizery.com
Date:   Tue Apr 16 20:34:49 2013 +0300

MAINTAINERS: add rpmsg entry

People and scripts look for this.

Signed-off-by: Ohad Ben-Cohen o...@wizery.com

diff --git a/MAINTAINERS b/MAINTAINERS
index 9561658..eaca6c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6604,6 +6604,14 @@ F:   drivers/remoteproc/
 F: Documentation/remoteproc.txt
 F: include/linux/remoteproc.h

+REMOTE PROCESSOR MESSAGING (RPMSG) SUBSYSTEM
+M: Ohad Ben-Cohen o...@wizery.com
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg.git
+S: Maintained
+F: drivers/rpmsg/
+F: Documentation/rpmsg.txt
+F: include/linux/rpmsg.h
+
 RFKILL
 M: Johannes Berg johan...@sipsolutions.net
 L: linux-wirel...@vger.kernel.org

Hope this all helps, please let me know if anything else is needed.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

adding rpmsg.git to linux next

[Ohad Ben-Cohen]

Hi Stephen,

Could you please add:

git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg.git#for-next

to linux-next to include new stuff coming from Rob?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] remoteproc fixes for 3.9

[Ohad Ben-Cohen]

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/remoteproc-3.9-fixes

for you to fetch changes up to c7426bce5933d16b492a34e42ae77e26fceddff6:

  remoteproc: fix FW_CONFIG typo (2013-04-07 15:11:27 +0300)


4 small remoteproc fixes:
- Suman fixed an issue that crawled in with the move to the new
  idr_alloc interface in 3.9
- Dmitry fixed an STE specific memory leak
- Sjur fixed an error path in the core
- Rob fixed a Kconfig typo


Dmitry Tarnyagin (1):
  remoteproc/ste: fix memory leak on shutdown

Robert Tivy (1):
  remoteproc: fix FW_CONFIG typo

Sjur Brændeland (1):
  remoteproc: fix error path of handle_vdev

Suman Anna (1):
  remoteproc: fix the error check for idr_alloc

 drivers/remoteproc/Kconfig   | 2 +-
 drivers/remoteproc/remoteproc_core.c | 6 --
 drivers/remoteproc/ste_modem_rproc.c | 7 ++-
 3 files changed, 11 insertions(+), 4 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] an hwspinlock fix for 3.9

[Ohad Ben-Cohen]

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/hwspinlock.git
tags/hwspinlock-3.9-fix

for you to fetch changes up to c10b90d85a5126d25c89cbaa50dc9fdd1c4d001a:

  hwspinlock: fix __hwspin_lock_request error path (2013-04-05 17:45:11 +0300)


An hwspinlock fix from Li Fei, taking care of a faulty error path.


Li Fei (1):
  hwspinlock: fix __hwspin_lock_request error path

 drivers/hwspinlock/hwspinlock_core.c | 2 ++
 1 file changed, 2 insertions(+)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remoteproc/omap: depend on omap4 or 5

[Ohad Ben-Cohen]

On Mon, Feb 18, 2013 at 1:06 PM, Vincent Stehlé v-ste...@ti.com wrote:
 Signed-off-by: Vincent Stehlé v-ste...@ti.com

Thanks, applied to remoteproc-next.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwspinlock: depend on OMAP4 or OMAP5

[Ohad Ben-Cohen]

On Wed, Feb 27, 2013 at 7:10 PM, Vincent Stehlé v-ste...@ti.com wrote:
 OMAP5 has spinlocks, too.

 Signed-off-by: Vincent Stehlé v-ste...@ti.com
 Cc: Ohad Ben-Cohen o...@wizery.com
 Cc: Tony Lindgren t...@atomide.com

Thanks, applied to hwspinlock-next.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/9] drivers: mailbox: framework creation

[Ohad Ben-Cohen]

Hi Omar,

On Fri, Dec 21, 2012 at 9:33 PM, Omar Ramirez Luna
omar.rami...@copitl.com wrote:
 Yes, I made the changes, for tidspbridge and remoteproc, I will submit
 both for review, based on this series.

Great, thanks.

Please note that when we do eventually merge this, we need your
updates to be squashed into Loic's patches so we don't break
bisectibility.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] omap: iommu: fixing NULL pointer return value

[Ohad Ben-Cohen]

Hi Guillaume,

You should also cc Joerg and the iommu ml for iommu patches (cc'ing now).

On Fri, Dec 21, 2012 at 3:36 PM, Guillaume Aubertin g-auber...@ti.com wrote:
 the returned NULL pointer is not detected by IS_ERR(), and then
 de-referenced in the calling function, omap_iommu_attach_dev().

 Signed-off-by: Guillaume Aubertin g-auber...@ti.com

Acked-by: Ohad Ben-Cohen o...@wizery.com

 ---
  drivers/iommu/omap-iommu.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
 index badc17c..a0fcad2 100644
 --- a/drivers/iommu/omap-iommu.c
 +++ b/drivers/iommu/omap-iommu.c
 @@ -861,7 +861,7 @@ static struct omap_iommu *omap_iommu_attach(const char 
 *name, u32 *iopgd)
 (void *)name,
 device_match_by_alias);
 if (!dev)
 -   return NULL;
 +   return ERR_PTR(-ENODEV);

 obj = to_iommu(dev);

 --
 1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] mailbox: rename omap_mbox in mailbox

[Ohad Ben-Cohen]

Hi Loic,

On Tue, Dec 18, 2012 at 3:10 PM, Loic Pallardy
loic.pallardy-...@stericsson.com wrote:
 In order to create a generic mailbox framework, functions
 and structures should be renamed in mailbox.

This patch should also update omap_remoteproc.c so the build doesn't
break (and if you have a panda board around it might be nice to run
the remoteproc sample to make sure things still generally work).

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/9] drivers: mailbox: framework creation

[Ohad Ben-Cohen]

On Thu, Dec 20, 2012 at 9:19 PM, Olof Johansson o...@lixom.net wrote:
 While we can make the branch stable, would it make sense to make
 remoteproc for omap depend on !multiplatform during the transition, to
 reduce dependencies a little? Either way works, but it'd be nice to
 keep them independent if we can.

I'm not sure multiplatform is the culprit; OMAP's remoteproc driver
heavily depends on this mailbox code, and obviously breaks with this
patch-set if only for the the naming changes. We'll need this patch
set to update omap's remoteproc as well so at least we don't break
bisectibility, though running a sanity test before merging would be
even nicer (Loic I can help if you don't have a panda board).

BTW - grep shows that tidspbridge is using the mailbox code too, but
it's in staging and I'm not sure it gets much love. Nevertheless, as
long as it's there we should at least update it with the new API as
well.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] remoteproc: a single fix for 3.7

[Ohad Ben-Cohen]

Hi Linus,

The following changes since commit 6f0c0580b70c89094b3422ba81118c7b959c7556:

  Linux 3.7-rc2 (2012-10-20 12:11:32 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/rproc-3.7-fix

for you to fetch changes up to dab55bbafdb491ce2c3f2d7136e54101e948b911:

  remoteproc: fix error path of -find_vqs (2012-11-29 10:05:09 +0200)


A single remoteproc fix for an error path issue reported by Ido Yariv.


Ohad Ben-Cohen (1):
  remoteproc: fix error path of -find_vqs

 drivers/remoteproc/remoteproc_virtio.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/5] OMAP: iommu: hwmod, reset handling and runtime PM

[Ohad Ben-Cohen]

On Tue, Nov 20, 2012 at 3:05 AM, Omar Ramirez Luna omar.l...@linaro.org wrote:
 These patches are needed for remoteproc to work on OMAP4.

 Introduced iommu hwmod support for OMAP3 (iva, isp) and
 OMAP4 (ipu, dsp), along with the corresponding runtime PM
 and routines to deassert reset lines, enable/disable clocks
 and configure sysc registers.

I tested the entire series with remoteproc/rpmsg on OMAP4 and it looks good.

Since this is super needed, and have already been going on for quite a
while, I'd really like to see this go in. We could then incrementally
deal with any outstanding comment or issue that might show up.

Joerg, can you please take it (or at least its first four patches) ?
We have Tony's ack for the mach-omap2 parts.

Tested-by: Ohad Ben-Cohen o...@wizery.com

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] remoteproc: fix error path of -find_vqs

[Ohad Ben-Cohen]

Eliminate an erroneous invocation of rproc_shutdown inside
the error path of rproc_virtio_find_vqs.

Reported-by: Ido Yariv i...@wizery.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 drivers/remoteproc/remoteproc_virtio.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c 
b/drivers/remoteproc/remoteproc_virtio.c
index e7a4780..9e198e5 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -120,15 +120,11 @@ static struct virtqueue *rp_find_vq(struct virtio_device 
*vdev,
return vq;
 }
 
-static void rproc_virtio_del_vqs(struct virtio_device *vdev)
+static void __rproc_virtio_del_vqs(struct virtio_device *vdev)
 {
struct virtqueue *vq, *n;
-   struct rproc *rproc = vdev_to_rproc(vdev);
struct rproc_vring *rvring;
 
-   /* power down the remote processor before deleting vqs */
-   rproc_shutdown(rproc);
-
list_for_each_entry_safe(vq, n, vdev-vqs, list) {
rvring = vq-priv;
rvring-vq = NULL;
@@ -137,6 +133,16 @@ static void rproc_virtio_del_vqs(struct virtio_device 
*vdev)
}
 }
 
+static void rproc_virtio_del_vqs(struct virtio_device *vdev)
+{
+   struct rproc *rproc = vdev_to_rproc(vdev);
+
+   /* power down the remote processor before deleting vqs */
+   rproc_shutdown(rproc);
+
+   __rproc_virtio_del_vqs(vdev);
+}
+
 static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs,
   struct virtqueue *vqs[],
   vq_callback_t *callbacks[],
@@ -163,7 +169,7 @@ static int rproc_virtio_find_vqs(struct virtio_device 
*vdev, unsigned nvqs,
return 0;
 
 error:
-   rproc_virtio_del_vqs(vdev);
+   __rproc_virtio_del_vqs(vdev);
return ret;
 }
 
-- 
1.7.10.rc3.1067.gb129051

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] ARM: OMAP3/4: iommu: adapt to runtime pm

[Ohad Ben-Cohen]

Hi Omar,

On Thu, Nov 15, 2012 at 6:53 PM, Omar Ramirez Luna omar.l...@linaro.org wrote:
 On 14 November 2012 03:54, Ohad Ben-Cohen o...@wizery.com wrote:
 Most of the above questions imply this patch not only converts the
 iommu to runtime PM, but may carry additional changes that may imply
 previous implementation is sub-optimal. I hope we can clearly document
 the motivation behind these changes too (maybe even consider
 extracting them to a different patch ?).

 I didn't want to extract this changes into different patches since
 they could be included in this one, otherwise it would look like lines
 adding and then deleting runtime pm functions.

 I do agree description is missing, again I thought I had done this
 somewhere but looks like I didn't, will update these. If you think
 these should be different patches please let me know, otherwise I
 would like to keep a single *documented* patch.

It seems like there are 3 different logical changes here:

1. remove clk_* invocations from iommu_fault_handler()
2. keep clocks enabled as long as iommu is enabled
3. convert to runtime pm

If we split this to three patches in this order, you won't have to add
and remove runtime pm functions.

Let's do it, please. It's a small nuisance now, but may be really
helpful in the future when someone tries to debug stuff and understand
the motivation behind these changes. It'd make it much easier for you
to document the changes, too: you have an entire commit log per
change.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] ARM: OMAP3/4: iommu: adapt to runtime pm

[Ohad Ben-Cohen]

Hi Omar,

On Wed, Nov 14, 2012 at 4:34 AM, Omar Ramirez Luna omar.l...@linaro.org wrote:
 Use runtime PM functionality interfaced with hwmod enable/idle
 functions, to replace direct clock operations and sysconfig
 handling.

 Dues to reset sequence, pm_runtime_put_sync must be used, to avoid
 possible operations with the module under reset.

There are some changes here that might not be trivial to understand in
hindsight; any chance you can add more explanations (even only in the
commit log) regarding:

 @@ -160,11 +160,10 @@ static int iommu_enable(struct omap_iommu *obj)
...
 -   clk_enable(obj-clk);
 +   pm_runtime_get_sync(obj-dev);

 err = arch_iommu-enable(obj);

 -   clk_disable(obj-clk);
 return err;
  }

Why do we remove clk_disable here (instead of replacing it with a _put
variant) ?

 @@ -306,7 +303,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, 
 struct iotlb_entry *e)
 if (!obj || !obj-nr_tlb_entries || !e)
 return -EINVAL;

 -   clk_enable(obj-clk);
 +   pm_runtime_get_sync(obj-dev);

If iommu_enable no longer disables obj-clk before returning, do we
really need to call -get here (and in all the other similar
instances) ?

 @@ -816,9 +813,7 @@ static irqreturn_t iommu_fault_handler(int irq, void 
 *data)
 if (!obj-refcount)
 return IRQ_NONE;

 -   clk_enable(obj-clk);
 errs = iommu_report_fault(obj, da);
 -   clk_disable(obj-clk);

Why do we remove the clk_ invocations here (instead of replacing them
with get/put variants) ?

Most of the above questions imply this patch not only converts the
iommu to runtime PM, but may carry additional changes that may imply
previous implementation is sub-optimal. I hope we can clearly document
the motivation behind these changes too (maybe even consider
extracting them to a different patch ?).

 @@ -990,6 +981,9 @@ static int __devinit omap_iommu_probe(struct 
 platform_device *pdev)
 goto err_irq;
 platform_set_drvdata(pdev, obj);

 +   pm_runtime_irq_safe(obj-dev);

Let's also document why _irq_safe is needed here ?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/6] Move rest of omap-iommu to live in drivers/iommu

[Ohad Ben-Cohen]

On Fri, Nov 2, 2012 at 9:23 PM, Tony Lindgren t...@atomide.com wrote:
 We need to move the iommu code to live under drivers
 for arm common zImage support.

For the iommu changes in the entire series:

Acked-by: Ohad Ben-Cohen o...@wizery.com

Joerg, it might relieve some pain if this will go through Tony's tree,
as there are some OMAP platform iommu changes coming in from Omar as
well (part of which might have already been merged in the omap
branches). Hope it's ok with both of you guys?

Omar, do you still have any iommu changes coming in ? Can we get
everything merged to the same tree ?

Thanks!
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] ARM: OMAP2+: Move plat/iovmm.h to include/linux/omap-iommu.h

[Ohad Ben-Cohen]

On Thu, Oct 25, 2012 at 11:39 PM, Tony Lindgren t...@atomide.com wrote:
  Joerg and Ohad, do you have any opinions on this?

I agree that there's some merit in having a separate header file for
IOVMM, since it's a different layer from the IOMMU API.

But in reality it's tightly coupled with OMAP's IOMMU, and ideally it
really should go away and be replaced with the DMA API.

For this reason, and for the fact that anyway there's only a single
user for it (omap3isp) and there will never be any more, maybe we
shouldn't pollute include/linux anymore.

Anyone volunteering to remove IOVMM and adapt omap3isp to the DMA API
instead ? ;)

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

On Wed, Oct 24, 2012 at 3:54 AM, Tony Lindgren t...@atomide.com wrote:
 I don't think mach-davinci has wl12xx_board_init() available?
 Maybe leave this out or define also also somewhere for mach-davinci?

Yeah let's leave davinci out for now:

From 665bcaa7ef0ed385cf1765f2d4503bf84aaf488a Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sun, 14 Oct 2012 20:16:01 +0200
Subject: [PATCH] ARM: OMAP: don't print any error when wl12xx isn't
 configured

Stop intimidating users with scary wlan error messages in case wl12xx
support wasn't even built.

In addition, when wl12xx_set_platform_data() fails, don't bother
registering wl12xx's fixed regulator device (on the relevant
boards).

While we're at it, extract the wlan init code to a dedicated function to
make (the relevant boards') init code look a bit nicer.

Compile tested only.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 arch/arm/mach-omap2/board-4430sdp.c  |  7 +++---
 arch/arm/mach-omap2/board-omap3evm.c |  6 ++---
 arch/arm/mach-omap2/board-omap3pandora.c |  9 +++-
 arch/arm/mach-omap2/board-omap4panda.c   | 20 -
 arch/arm/mach-omap2/board-zoom-peripherals.c | 15 -
 arch/arm/mach-omap2/common-board-devices.c   | 33 
 arch/arm/mach-omap2/common-board-devices.h   |  2 ++
 7 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c
b/arch/arm/mach-omap2/board-4430sdp.c
index 3669c12..bc48679 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -825,10 +825,11 @@ static void __init omap4_sdp4430_wifi_init(void)
int ret;

omap4_sdp4430_wifi_mux_init();
-   omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
-   ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
+
+   ret = wl12xx_board_init(omap4_sdp4430_wlan_data, GPIO_WIFI_IRQ);
if (ret)
-   pr_err(Error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap_vwlan_device);
if (ret)
pr_err(Error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index b9b776b..8e3a075 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -636,10 +636,10 @@ static void __init omap3_evm_wl12xx_init(void)
int ret;

/* WL12xx WLAN Init */
-   omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
-   ret = wl12xx_set_platform_data(omap3evm_wlan_data);
+   ret = wl12xx_board_init(omap3evm_wlan_data, OMAP3EVM_WLAN_IRQ_GPIO);
if (ret)
-   pr_err(error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap3evm_wlan_regulator);
if (ret)
pr_err(error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c
b/arch/arm/mach-omap2/board-omap3pandora.c
index 00a1f4a..bfdc7aa 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -543,13 +543,10 @@ static void __init pandora_wl1251_init(void)
if (ret  0)
goto fail;

-   pandora_wl1251_pdata.irq = gpio_to_irq(PANDORA_WIFI_IRQ_GPIO);
-   if (pandora_wl1251_pdata.irq  0)
-   goto fail_irq;
-
pandora_wl1251_pdata.use_eeprom = true;
-   ret = wl12xx_set_platform_data(pandora_wl1251_pdata);
-   if (ret  0)
+
+   ret = wl12xx_board_init(pandora_wl1251_pdata, PANDORA_WIFI_IRQ_GPIO);
+   if (ret)
goto fail_irq;

return;
diff --git a/arch/arm/mach-omap2/board-omap4panda.c
b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..f203cd9 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -486,24 +486,32 @@ static void omap4_panda_init_rev(void)
}
 }

+static void __init omap4_panda_wlan_init(void)
+{
+   int ret;
+
+   ret = wl12xx_board_init(omap_panda_wlan_data, GPIO_WIFI_IRQ);
+   if (ret)
+   return;
+
+   ret = platform_device_register(omap_vwlan_device);
+   if (ret)
+   pr_err(error registering wl12xx's fixed regulator: %d\n, ret);
+}
+
 static void __init omap4_panda_init(void)
 {
int package = OMAP_PACKAGE_CBS;
-   int ret;

if (omap_rev() == OMAP4430_REV_ES1_0)
package = OMAP_PACKAGE_CBL;
omap4_mux_init(board_mux, NULL, package);

-   omap_panda_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
-   ret = wl12xx_set_platform_data(omap_panda_wlan_data);
-   if (ret)
-   pr_err(error setting wl12xx data: %d\n, ret);
+   omap4_panda_wlan_init();

omap4_panda_init_rev();
omap4_panda_i2c_init();
platform_add_devices(panda_devices

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

On Tue, Oct 23, 2012 at 9:37 AM, Igor Grinberg grinb...@compulab.co.il wrote:
 + ret = wl12xx_set_platform_data(wlan_data);
 + /* bail out silently in case wl12xx isn't configured */
 + if (ret == -ENOSYS)
 + return ret;

 Since we have the function ifdef'ed, I don't think we need
 the ENOSYS check, do we?

If we want to be strict, we better not remove it.

It's an interface that hides the internal implementation, and it's
just better not to assume anything beyond the return values and their
meanings. This way if WLAN folks change something in the future, we
don't need to update all the boards code again.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

On Fri, Oct 19, 2012 at 7:07 PM, Tony Lindgren t...@atomide.com wrote:
...
 We could optimize away a minimal amount of code for many configurations
 with the ifdef? :)

Sure, here goes (compile tested only):

From 6b82365da2c04986e647d06c285197efece7cb34 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sun, 14 Oct 2012 20:16:01 +0200
Subject: [PATCH] ARM: OMAP: don't print any error when wl12xx isn't
 configured

Stop intimidating users with scary wlan error messages in case wl12xx
support wasn't even built.

In addition, when wl12xx_set_platform_data() fails, don't bother
registering wl12xx's fixed regulator device (on the relevant
boards).

While we're at it, extract the wlan init code to a dedicated function to
make (the relevant boards') init code look a bit nicer.

Compile tested only.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 arch/arm/mach-davinci/board-da850-evm.c  |  8 ++-
 arch/arm/mach-omap2/board-4430sdp.c  |  7 +++---
 arch/arm/mach-omap2/board-omap3evm.c |  6 ++---
 arch/arm/mach-omap2/board-omap3pandora.c |  9 +++-
 arch/arm/mach-omap2/board-omap4panda.c   | 20 -
 arch/arm/mach-omap2/board-zoom-peripherals.c | 15 -
 arch/arm/mach-omap2/common-board-devices.c   | 33 
 arch/arm/mach-omap2/common-board-devices.h   |  2 ++
 8 files changed, 71 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index 32ee3f8..7243c22 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1401,13 +1401,9 @@ static __init int da850_wl12xx_init(void)
goto free_wlan_en;
}

-   da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
-
-   ret = wl12xx_set_platform_data(da850_wl12xx_wlan_data);
-   if (ret) {
-   pr_err(Could not set wl12xx data: %d\n, ret);
+   ret = wl12xx_board_init(da850_wl12xx_wlan_data, DA850_WLAN_IRQ);
+   if (ret)
goto free_wlan_irq;
-   }

return 0;

diff --git a/arch/arm/mach-omap2/board-4430sdp.c
b/arch/arm/mach-omap2/board-4430sdp.c
index 3669c12..bc48679 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -825,10 +825,11 @@ static void __init omap4_sdp4430_wifi_init(void)
int ret;

omap4_sdp4430_wifi_mux_init();
-   omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
-   ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
+
+   ret = wl12xx_board_init(omap4_sdp4430_wlan_data, GPIO_WIFI_IRQ);
if (ret)
-   pr_err(Error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap_vwlan_device);
if (ret)
pr_err(Error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index b9b776b..8e3a075 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -636,10 +636,10 @@ static void __init omap3_evm_wl12xx_init(void)
int ret;

/* WL12xx WLAN Init */
-   omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
-   ret = wl12xx_set_platform_data(omap3evm_wlan_data);
+   ret = wl12xx_board_init(omap3evm_wlan_data, OMAP3EVM_WLAN_IRQ_GPIO);
if (ret)
-   pr_err(error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap3evm_wlan_regulator);
if (ret)
pr_err(error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c
b/arch/arm/mach-omap2/board-omap3pandora.c
index 00a1f4a..bfdc7aa 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -543,13 +543,10 @@ static void __init pandora_wl1251_init(void)
if (ret  0)
goto fail;

-   pandora_wl1251_pdata.irq = gpio_to_irq(PANDORA_WIFI_IRQ_GPIO);
-   if (pandora_wl1251_pdata.irq  0)
-   goto fail_irq;
-
pandora_wl1251_pdata.use_eeprom = true;
-   ret = wl12xx_set_platform_data(pandora_wl1251_pdata);
-   if (ret  0)
+
+   ret = wl12xx_board_init(pandora_wl1251_pdata, PANDORA_WIFI_IRQ_GPIO);
+   if (ret)
goto fail_irq;

return;
diff --git a/arch/arm/mach-omap2/board-omap4panda.c
b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..f203cd9 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -486,24 +486,32 @@ static void omap4_panda_init_rev(void)
}
 }

+static void __init omap4_panda_wlan_init(void)
+{
+   int ret;
+
+   ret = wl12xx_board_init(omap_panda_wlan_data, GPIO_WIFI_IRQ);
+   if (ret)
+   return;
+
+   ret

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

Hi Igor,

On Wed, Oct 17, 2012 at 2:43 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 You are adding declarations inside the common-board-devices.h,
 but the implementation inside the devices.c.
 I can see that devices.c has only on-soc devices in it.
 So, I would expect to have the wl12xx_board_init() function implementation
 inside the common-board-devices.c file.

I really don't mind. Tony do you have any preference?

 Another minor nit: I don't think you need to mark the declaration as __init,
 only the implementation, or is it for documenting it so?

It may be, but I don't really mind removing it. Let's remove it if
we'll move to common-board-devices.c, otherwise it probably isn't
worth the noise.

 Instead of the above, wouldn't it be better to have:
 #if defined(CONFIG_WL12XX_PLATFORM_DATA)
 int __init wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int 
 irq_gpio)
 {
 ...
 }
 #else
 inline int wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int 
 irq_gpio)
 {
 return 0;
 }
 #endif

I think readability-wise we're probably better off without the #ifdef.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

On Tue, Oct 16, 2012 at 8:26 PM, Tony Lindgren t...@atomide.com wrote:
 Hmm looking at it repeats the same code over again. Can you
 rather add some wl12xx_board_init() helper function to mach-omap2/devices.c
 to do it?

Nice, see below. Note that I can only compile test this now, which may
be ok because it's pretty trivial. But do let me know if you want me
to get it tested.

From b940fb88a97494ad3a0a093279a5f176c0b29e44 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sun, 14 Oct 2012 20:16:01 +0200
Subject: [PATCH] ARM: OMAP: don't print any error when wl12xx isn't
 configured

Stop intimidating users with scary wlan error messages in case wl12xx
support wasn't even built.

In addition, when wl12xx_set_platform_data() fails, don't bother
registering wl12xx's fixed regulator device (on the relevant
boards).

While we're at it, extract the wlan init code to a dedicated function to
make (the relevant boards') init code look a bit nicer.

Compile tested only.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 arch/arm/mach-davinci/board-da850-evm.c  |  8 ++--
 arch/arm/mach-omap2/board-4430sdp.c  |  7 ---
 arch/arm/mach-omap2/board-omap3evm.c |  6 +++---
 arch/arm/mach-omap2/board-omap3pandora.c |  9 +++--
 arch/arm/mach-omap2/board-omap4panda.c   | 20 ++--
 arch/arm/mach-omap2/board-zoom-peripherals.c | 15 ++-
 arch/arm/mach-omap2/common-board-devices.h   |  2 ++
 arch/arm/mach-omap2/devices.c| 26 ++
 8 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index 32ee3f8..7243c22 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1401,13 +1401,9 @@ static __init int da850_wl12xx_init(void)
goto free_wlan_en;
}

-   da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
-
-   ret = wl12xx_set_platform_data(da850_wl12xx_wlan_data);
-   if (ret) {
-   pr_err(Could not set wl12xx data: %d\n, ret);
+   ret = wl12xx_board_init(da850_wl12xx_wlan_data, DA850_WLAN_IRQ);
+   if (ret)
goto free_wlan_irq;
-   }

return 0;

diff --git a/arch/arm/mach-omap2/board-4430sdp.c
b/arch/arm/mach-omap2/board-4430sdp.c
index 3669c12..bc48679 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -825,10 +825,11 @@ static void __init omap4_sdp4430_wifi_init(void)
int ret;

omap4_sdp4430_wifi_mux_init();
-   omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
-   ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
+
+   ret = wl12xx_board_init(omap4_sdp4430_wlan_data, GPIO_WIFI_IRQ);
if (ret)
-   pr_err(Error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap_vwlan_device);
if (ret)
pr_err(Error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index b9b776b..8e3a075 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -636,10 +636,10 @@ static void __init omap3_evm_wl12xx_init(void)
int ret;

/* WL12xx WLAN Init */
-   omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
-   ret = wl12xx_set_platform_data(omap3evm_wlan_data);
+   ret = wl12xx_board_init(omap3evm_wlan_data, OMAP3EVM_WLAN_IRQ_GPIO);
if (ret)
-   pr_err(error setting wl12xx data: %d\n, ret);
+   return;
+
ret = platform_device_register(omap3evm_wlan_regulator);
if (ret)
pr_err(error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c
b/arch/arm/mach-omap2/board-omap3pandora.c
index 00a1f4a..bfdc7aa 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -543,13 +543,10 @@ static void __init pandora_wl1251_init(void)
if (ret  0)
goto fail;

-   pandora_wl1251_pdata.irq = gpio_to_irq(PANDORA_WIFI_IRQ_GPIO);
-   if (pandora_wl1251_pdata.irq  0)
-   goto fail_irq;
-
pandora_wl1251_pdata.use_eeprom = true;
-   ret = wl12xx_set_platform_data(pandora_wl1251_pdata);
-   if (ret  0)
+
+   ret = wl12xx_board_init(pandora_wl1251_pdata, PANDORA_WIFI_IRQ_GPIO);
+   if (ret)
goto fail_irq;

return;
diff --git a/arch/arm/mach-omap2/board-omap4panda.c
b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..f203cd9 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -486,24 +486,32 @@ static void omap4_panda_init_rev(void)
}
 }

+static void __init

Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)

[Ohad Ben-Cohen]

On Fri, Oct 12, 2012 at 6:24 PM, Tony Lindgren t...@atomide.com wrote:
 Error setting wl12xx data: -38
..
 Ohad, can you please take a look?

Sure, -38 is -ENOSYS which is returned when the wl12xx driver isn't configured.

This isn't an error (it's a user decision, and it shouldn't elicit any
error) and the patch below (also attached) should make it go away on:

From 374f145568585c8d6a8d5e4b8b5d3e6baedd2f39 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sun, 14 Oct 2012 20:16:01 +0200
Subject: [PATCH] ARM: OMAP: don't print any error when wl12xx isn't
 configured

Stop intimidating users with scary wlan error messages in case wl12xx
support wasn't even built.

In addition, when wl12xx_set_platform_data() fails, don't bother
registering wl12xx's fixed regulator device (on the relevant
boards).

While we're at it, extract the wlan init code to a dedicated function to
make (the relevant boards') init code look a bit nicer.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 arch/arm/mach-davinci/board-da850-evm.c  |  7 ++-
 arch/arm/mach-omap2/board-4430sdp.c  | 13 ++--
 arch/arm/mach-omap2/board-omap3evm.c | 11 +-
 arch/arm/mach-omap2/board-omap3pandora.c |  9 -
 arch/arm/mach-omap2/board-omap4panda.c   | 30 ++--
 arch/arm/mach-omap2/board-zoom-peripherals.c | 20 ---
 6 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index 32ee3f8..1b19415 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1404,8 +1404,13 @@ static __init int da850_wl12xx_init(void)
da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);

ret = wl12xx_set_platform_data(da850_wl12xx_wlan_data);
+   /* bail out silently in case wl12xx isn't configured */
+   if (ret == -ENOSYS)
+   goto free_wlan_irq;
+
+   /* bail out verbosely on any other error */
if (ret) {
-   pr_err(Could not set wl12xx data: %d\n, ret);
+   pr_err(error setting wl12xx data: %d\n, ret);
goto free_wlan_irq;
}

diff --git a/arch/arm/mach-omap2/board-4430sdp.c
b/arch/arm/mach-omap2/board-4430sdp.c
index 3669c12..d1dd81a 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -826,9 +826,18 @@ static void __init omap4_sdp4430_wifi_init(void)

omap4_sdp4430_wifi_mux_init();
omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
+
ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
-   if (ret)
-   pr_err(Error setting wl12xx data: %d\n, ret);
+   /* bail out silently in case wl12xx isn't configured */
+   if (ret == -ENOSYS)
+   return;
+
+   /* bail out verbosely on any other error */
+   if (ret) {
+   pr_err(error setting wl12xx data: %d\n, ret);
+   return;
+   }
+
ret = platform_device_register(omap_vwlan_device);
if (ret)
pr_err(Error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index b9b776b..7f72e44 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -637,9 +637,18 @@ static void __init omap3_evm_wl12xx_init(void)

/* WL12xx WLAN Init */
omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
+
ret = wl12xx_set_platform_data(omap3evm_wlan_data);
-   if (ret)
+   /* bail out silently in case wl12xx isn't configured */
+   if (ret == -ENOSYS)
+   return;
+
+   /* bail out verbosely on any other error */
+   if (ret) {
pr_err(error setting wl12xx data: %d\n, ret);
+   return;
+   }
+
ret = platform_device_register(omap3evm_wlan_regulator);
if (ret)
pr_err(error registering wl12xx device: %d\n, ret);
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c
b/arch/arm/mach-omap2/board-omap3pandora.c
index 00a1f4a..bbc4001 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -549,8 +549,15 @@ static void __init pandora_wl1251_init(void)

pandora_wl1251_pdata.use_eeprom = true;
ret = wl12xx_set_platform_data(pandora_wl1251_pdata);
-   if (ret  0)
+   /* bail out silently in case wl12xx isn't configured */
+   if (ret == -ENOSYS)
+   goto fail_irq;
+
+   /* bail out verbosely on any other error */
+   if (ret) {
+   pr_err(error setting wl12xx data: %d\n, ret);
goto fail_irq;
+   }

return;

diff --git a/arch/arm/mach-omap2/board-omap4panda.c
b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..cc5cdd6 100644

[GIT PULL] remoteproc for 3.7

[Ohad Ben-Cohen]

Hi Linus,

Please pull remoteproc 3.7 material, thanks.

PS - I recently found an annoying typo in the subject of one of the
patches, but I decided to accept it over doing a rebase.

The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217:

  Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git
tags/remoteproc-for-3.7

for you to fetch changes up to d09f53a735bae43806a77754312a45d3f1198342:

  remoteproc: Fix use of format specifyer (2012-10-02 10:14:43 +0200)


- Remoteproc Recovery - by Fernando Guzman Lugo - when a remote processor
  crash is detected, this mechanism will remove all virtio children
  devices, wait until their drivers let go, hard reset the remote
  processor and reload the firmware (resulting in the relevant virtio
  children devices re-added). Essentially the entire software stack
  is reset, together with the relevant hardware, so users don't have
  to reset the entire phone.
- STE Modem driver is added - by Sjur Brændeland
- OMAP DSP boot address support is added - by Juan Gutierrez
- A handful of fixes/cleanups - Sjur Brændeland, Dan Carpenter, Emil Goode


Dan Carpenter (3):
  remoteproc: snprintf() can return more than was printed
  remoteproc: return -EFAULT on copy_from_user failure
  remoteproc: fix a potential NULL-dereference on cleanup

Emil Goode (1):
  remoteproc: Fix use of format specifyer

Fernando Guzman Lugo (3):
  remoteproc: add rproc_report_crash function to notify rproc crashes
  remoteproc: add actual recovery implementation
  remoteproc: create a 'recovery' debugfs entry

Juan Gutierrez (1):
  remoteproc/omap: set bootaddr support

Ohad Ben-Cohen (1):
  remoteproc: select VIRTIO to avoid build breakage

Sjur Brændeland (3):
  remoteproc: Add dependency to HAS_DMA
  remtoteproc: maintain max notifyid
  remoteproc: Add STE modem driver

 Documentation/remoteproc.txt |   7 +
 arch/arm/plat-omap/include/plat/remoteproc.h |   2 +
 drivers/remoteproc/Kconfig   |  14 ++
 drivers/remoteproc/Makefile  |   1 +
 drivers/remoteproc/omap_remoteproc.c |   3 +
 drivers/remoteproc/remoteproc_core.c | 209 +
 drivers/remoteproc/remoteproc_debugfs.c  |  85 ++-
 drivers/remoteproc/remoteproc_internal.h |   1 +
 drivers/remoteproc/ste_modem_rproc.c | 322 +++
 include/linux/remoteproc.h   |  24 ++
 include/linux/ste_modem_shm.h|  56 +
 11 files changed, 682 insertions(+), 42 deletions(-)
 create mode 100644 drivers/remoteproc/ste_modem_rproc.c
 create mode 100644 include/linux/ste_modem_shm.h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html