linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: acourbot@nvidia.com (Alex Courbot)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: move firmware_ops to drivers/firmware
Date: Mon, 18 Nov 2013 12:05:59 +0900	[thread overview]
Message-ID: <52898417.80601@nvidia.com> (raw)
In-Reply-To: <CAHkRjk5LBOFx3ir49RgEsaFwvp5UCpTjYFWto+URXebAsOP0RA@mail.gmail.com>

On 11/18/2013 12:59 AM, Catalin Marinas wrote:
> On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote:
>> The ARM tree includes a firmware_ops interface that is designed to
>> implement support for simple, TrustZone-based firmwares but could
>> also cover other use-cases. It has been suggested that this
>> interface might be useful to other architectures (e.g. arm64) and
>> that it should be moved out of arch/arm.
>
> NAK. I'm for code sharing with arm via common locations but this API
> goes against the ARMv8 firmware standardisation efforts like PSCI,
> encouraging each platform to define there own non-standard interface.

I have to say, I pretty much agree with your NAK.

The reason for this patch is that the suggestion to move firmware_ops 
out of arch/arm is the last (I hope) thing that prevents my Trusted 
Foundation support series from being merged. Now if we can all agree:

* that ARMv8 will only use PSCI
* that there is no use-case of this interface outside of arch/arm as of 
today (and none foreseen in the near future)
* that the firmware_ops interface is quite ARMv7-specific anyway,
* that should a need to move it (for whatever reason) occur later, it 
will be easy to do (as this patch hopefully demonstrates).

... then this has indeed no reason to be. And maybe I can finally get 
Russell's blessing on my series.

>
>> --- /dev/null
>> +++ b/include/linux/platform_firmware.h
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics.
>> + * Kyungmin Park <kyungmin.park@samsung.com>
>> + * Tomasz Figa <t.figa@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _PLATFORM_FIRMWARE_H
>> +#define _PLATFORM_FIRMWARE_H
>> +
>> +#include <linux/bug.h>
>> +
>> +/*
>> + * struct platform_firmware_ops
>> + *
>> + * A structure to specify available firmware operations.
>> + *
>> + * A filled up structure can be registered with
>> + * register_platform_firmware_ops().
>> + */
>> +struct platform_firmware_ops {
>> +       /*
>> +        * Enters CPU idle mode
>> +        */
>> +       int (*do_idle)(void);
>
> Covered by PSCI already.
>
>> +       /*
>> +        * Sets boot address of specified physical CPU
>> +        */
>> +       int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
>
> Covered either by PSCI or spin-table release method (PSCI if firmware
> call is required).
>
>> +       /*
>> +        * Boots specified physical CPU
>> +        */
>> +       int (*cpu_boot)(int cpu);
>
> PSCI.
>
>> +       /*
>> +        * Initializes L2 cache
>> +        */
>> +       int (*l2x0_init)(void);
>
> No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly
> recommend the hardware people to make proper external caches which can
> be flushed by standard CPU instructions, not MMIO. Any such caches
> must be enabled by firmware before Linux starts.
>
> The above firmware API is 32-bit ARM only. Form 64-bit ARM, you have
> the choice of PSCI so far but as I said in a long thread to Nico, I'm
> open to other standard interfaces if there are good reasons PSCI
> cannot be used. Note the _standard_ part, I don't want every SoC with
> their own firmware API for standard things like secondary CPU
> booting/hotplug/idle.
>

  reply	other threads:[~2013-11-18  3:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-17  8:49 [PATCH] ARM: move firmware_ops to drivers/firmware Alexandre Courbot
2013-11-17 15:59 ` Catalin Marinas
2013-11-18  3:05   ` Alex Courbot [this message]
2013-11-18 11:58     ` Catalin Marinas
2013-11-18 17:03       ` Stephen Warren
2013-11-18 17:10         ` Russell King - ARM Linux
2013-11-18 17:18           ` Stephen Warren
2013-11-18 17:30         ` Catalin Marinas
2013-11-18 17:52           ` Stephen Warren
2013-11-19 11:38             ` Catalin Marinas
2013-11-18 19:04           ` Christopher Covington
2013-11-19 11:02             ` Catalin Marinas
2013-11-19  2:46       ` Alex Courbot
2013-11-19 12:26         ` Catalin Marinas
2013-11-19 14:29           ` Alexandre Courbot
2013-11-19 15:07             ` Catalin Marinas
2013-11-19 15:17               ` Alexandre Courbot
2013-11-18 17:00   ` Stephen Warren
2013-11-18 17:23     ` Catalin Marinas
2013-11-18 17:14   ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52898417.80601@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).