Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1440650 > unrolled thread

Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add bindings of the BPMP

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra:  add bindings of the BPMP Rob Herring <robh@kernel.org> - 2016-07-11 16:30 +0200
    Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add  bindings of the BPMP Stephen Warren <swarren@wwwdotorg.org> - 2016-07-11 18:10 +0200
      Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add  bindings of the BPMP Joseph Lo <josephl@nvidia.com> - 2016-07-18 09:50 +0200
        Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add  bindings of the BPMP Stephen Warren <swarren@wwwdotorg.org> - 2016-07-18 18:20 +0200

#1440650 — Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add bindings of the BPMP

On Tue, Jul 05, 2016 at 05:04:24PM +0800, Joseph Lo wrote:
> The BPMP is a specific processor in Tegra chip, which is designed for
> booting process handling and offloading the power management, clock
> management, and reset control tasks from the CPU. The binding document
> defines the resources that would be used by the BPMP firmware driver,
> which can create the interprocessor communication (IPC) between the CPU
> and BPMP.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> Changes in V2:
> - update the message that the BPMP is clock and reset control provider
> - add tegra186-clock.h and tegra186-reset.h header files
> - revise the description of the required properties
> ---
>  .../bindings/firmware/nvidia,tegra186-bpmp.txt     |  77 ++
>  include/dt-bindings/clock/tegra186-clock.h         | 940 +++++++++++++++++++++
>  include/dt-bindings/reset/tegra186-reset.h         | 217 +++++
>  3 files changed, 1234 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
>  create mode 100644 include/dt-bindings/clock/tegra186-clock.h
>  create mode 100644 include/dt-bindings/reset/tegra186-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
> new file mode 100644
> index 000000000000..4d0b6eba56c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
> @@ -0,0 +1,77 @@
> +NVIDIA Tegra Boot and Power Management Processor (BPMP)
> +
> +The BPMP is a specific processor in Tegra chip, which is designed for
> +booting process handling and offloading the power management, clock
> +management, and reset control tasks from the CPU. The binding document
> +defines the resources that would be used by the BPMP firmware driver,
> +which can create the interprocessor communication (IPC) between the CPU
> +and BPMP.
> +
> +Required properties:
> +- name : Should be bpmp
> +- compatible
> +    Array of strings
> +    One of:
> +    - "nvidia,tegra186-bpmp"
> +- mboxes : The phandle of mailbox controller and the mailbox specifier.
> +- shmem : List of the phandle of the TX and RX shared memory area that
> +	  the IPC between CPU and BPMP is based on.

I think you can use memory-region here.

> +- #clock-cells : Should be 1.
> +- #reset-cells : Should be 1.
> +
> +This node is a mailbox consumer. See the following files for details of
> +the mailbox subsystem, and the specifiers implemented by the relevant
> +provider(s):
> +
> +- Documentation/devicetree/bindings/mailbox/mailbox.txt
> +- Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
> +
> +This node is a clock and reset provider. See the following files for
> +general documentation of those features, and the specifiers implemented
> +by this node:
> +
> +- Documentation/devicetree/bindings/clock/clock-bindings.txt
> +- include/dt-bindings/clock/tegra186-clock.h
> +- Documentation/devicetree/bindings/reset/reset.txt
> +- include/dt-bindings/reset/tegra186-reset.h
> +
> +The shared memory bindings for BPMP
> +-----------------------------------
> +
> +The shared memory area for the IPC TX and RX between CPU and BPMP are
> +predefined and work on top of sysram, which is an SRAM inside the chip.
> +
> +See "Documentation/devicetree/bindings/sram/sram.txt" for the bindings.
> +
> +Example:
> +
> +hsp_top0: hsp@03c00000 {
> +	...
> +	#mbox-cells = <1>;
> +};
> +
> +sysram@30000000 {
> +	compatible = "nvidia,tegra186-sysram", "mmio-ram";
> +	reg = <0x0 0x30000000 0x0 0x50000>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	ranges = <0 0x0 0x0 0x30000000 0x0 0x50000>;
> +
> +	cpu_bpmp_tx: bpmp_shmem@4e000 {
> +		compatible = "nvidia,tegra186-bpmp-shmem";
> +		reg = <0x0 0x4e000 0x0 0x1000>;
> +	};
> +
> +	cpu_bpmp_rx: bpmp_shmem@4f000 {
> +		compatible = "nvidia,tegra186-bpmp-shmem";
> +		reg = <0x0 0x4f000 0x0 0x1000>;
> +	};
> +};
> +
> +bpmp {
> +	compatible = "nvidia,tegra186-bpmp";
> +	mboxes = <&hsp_top0 HSP_MBOX_ID(DB, HSP_DB_MASTER_BPMP)>;
> +	shmem = <&cpu_bpmp_tx &cpu_bpmp_rx>;
> +	#clock-cells = <1>;
> +	#reset-cells = <1>;
> +};
> diff --git a/include/dt-bindings/clock/tegra186-clock.h b/include/dt-bindings/clock/tegra186-clock.h
> new file mode 100644
> index 000000000000..f73d32098f99
> --- /dev/null
> +++ b/include/dt-bindings/clock/tegra186-clock.h
> @@ -0,0 +1,940 @@
> +/** @file */
> +
> +#ifndef _MACH_T186_CLK_T186_H
> +#define _MACH_T186_CLK_T186_H
> +
> +/**
> + * @defgroup clock_ids Clock Identifiers

Aren't these doxygen markup? Does that work with docbook? If not, 
remove.

[toc] | [next] | [standalone]

#1440735 — Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add bindings of the BPMP

On 07/11/2016 08:22 AM, Rob Herring wrote:
> On Tue, Jul 05, 2016 at 05:04:24PM +0800, Joseph Lo wrote:
>> The BPMP is a specific processor in Tegra chip, which is designed for
>> booting process handling and offloading the power management, clock
>> management, and reset control tasks from the CPU. The binding document
>> defines the resources that would be used by the BPMP firmware driver,
>> which can create the interprocessor communication (IPC) between the CPU
>> and BPMP.

>> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt

>> +NVIDIA Tegra Boot and Power Management Processor (BPMP)
>> +
>> +The BPMP is a specific processor in Tegra chip, which is designed for
>> +booting process handling and offloading the power management, clock
>> +management, and reset control tasks from the CPU. The binding document
>> +defines the resources that would be used by the BPMP firmware driver,
>> +which can create the interprocessor communication (IPC) between the CPU
>> +and BPMP.
>> +
>> +Required properties:
>> +- name : Should be bpmp
>> +- compatible
>> +    Array of strings
>> +    One of:
>> +    - "nvidia,tegra186-bpmp"
>> +- mboxes : The phandle of mailbox controller and the mailbox specifier.
>> +- shmem : List of the phandle of the TX and RX shared memory area that
>> +	  the IPC between CPU and BPMP is based on.
>
> I think you can use memory-region here.

Isn't memory-region intended for references into the /reserved-memory 
node. If so, that isn't appropriate in this case since this property 
typically points at on-chip SRAM that isn't included in the OS's view of 
"system RAM".

Or, should /reserved-memory be used even for (e.g. non-DRAM) memory 
regions that aren't represented by the /memory/reg property?

>> diff --git a/include/dt-bindings/clock/tegra186-clock.h b/include/dt-bindings/clock/tegra186-clock.h

>> +/** @file */
>> +
>> +#ifndef _MACH_T186_CLK_T186_H
>> +#define _MACH_T186_CLK_T186_H
>> +
>> +/**
>> + * @defgroup clock_ids Clock Identifiers
>
> Aren't these doxygen markup? Does that work with docbook? If not,
> remove.

These headers are part of the BPMP FW release. It's preferable not to 
edit them when incorporating them into the Linux kernel (or any other SW 
stack) to simplify integration of any updated versions of the header, by 
removing the need to edit the file when doing so. Given that, do you 
still object?

[toc] | [prev] | [next] | [standalone]

#1445280 — Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add bindings of the BPMP

Hi Rob,

Thanks for your reviewing.

On 07/12/2016 12:05 AM, Stephen Warren wrote:
> On 07/11/2016 08:22 AM, Rob Herring wrote:
>> On Tue, Jul 05, 2016 at 05:04:24PM +0800, Joseph Lo wrote:
>>> The BPMP is a specific processor in Tegra chip, which is designed for
>>> booting process handling and offloading the power management, clock
>>> management, and reset control tasks from the CPU. The binding document
>>> defines the resources that would be used by the BPMP firmware driver,
>>> which can create the interprocessor communication (IPC) between the CPU
>>> and BPMP.
>
>>> diff --git
>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
>
>>> +NVIDIA Tegra Boot and Power Management Processor (BPMP)
>>> +
>>> +The BPMP is a specific processor in Tegra chip, which is designed for
>>> +booting process handling and offloading the power management, clock
>>> +management, and reset control tasks from the CPU. The binding document
>>> +defines the resources that would be used by the BPMP firmware driver,
>>> +which can create the interprocessor communication (IPC) between the CPU
>>> +and BPMP.
>>> +
>>> +Required properties:
>>> +- name : Should be bpmp
>>> +- compatible
>>> +    Array of strings
>>> +    One of:
>>> +    - "nvidia,tegra186-bpmp"
>>> +- mboxes : The phandle of mailbox controller and the mailbox specifier.
>>> +- shmem : List of the phandle of the TX and RX shared memory area that
>>> +      the IPC between CPU and BPMP is based on.
>>
>> I think you can use memory-region here.
>
> Isn't memory-region intended for references into the /reserved-memory
> node. If so, that isn't appropriate in this case since this property
> typically points at on-chip SRAM that isn't included in the OS's view of
> "system RAM".
Agree with that.

>
> Or, should /reserved-memory be used even for (e.g. non-DRAM) memory
> regions that aren't represented by the /memory/reg property?
>

For shmem, I follow the same concept of the binding for arm,scpi 
(.../arm/arm,scpi.txt) that is currently using in mainline. Do you think 
that is more appropriate here?


>>> diff --git a/include/dt-bindings/clock/tegra186-clock.h
>>> b/include/dt-bindings/clock/tegra186-clock.h
>
>>> +/** @file */
>>> +
>>> +#ifndef _MACH_T186_CLK_T186_H
>>> +#define _MACH_T186_CLK_T186_H
>>> +
>>> +/**
>>> + * @defgroup clock_ids Clock Identifiers
>>
>> Aren't these doxygen markup? Does that work with docbook? If not,
>> remove.
>
> These headers are part of the BPMP FW release. It's preferable not to
> edit them when incorporating them into the Linux kernel (or any other SW
> stack) to simplify integration of any updated versions of the header, by
> removing the need to edit the file when doing so. Given that, do you
> still object?

How do you think of this, Rob?

Thanks,
-Joseph

[toc] | [prev] | [next] | [standalone]

#1445614 — Re: [PATCH V2 03/10] Documentation: dt-bindings: firmware: tegra: add bindings of the BPMP

On 07/18/2016 01:44 AM, Joseph Lo wrote:
> Hi Rob,
>
> Thanks for your reviewing.
>
> On 07/12/2016 12:05 AM, Stephen Warren wrote:
>> On 07/11/2016 08:22 AM, Rob Herring wrote:
>>> On Tue, Jul 05, 2016 at 05:04:24PM +0800, Joseph Lo wrote:
>>>> The BPMP is a specific processor in Tegra chip, which is designed for
>>>> booting process handling and offloading the power management, clock
>>>> management, and reset control tasks from the CPU. The binding document
>>>> defines the resources that would be used by the BPMP firmware driver,
>>>> which can create the interprocessor communication (IPC) between the CPU
>>>> and BPMP.
>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
>>
>>>> +NVIDIA Tegra Boot and Power Management Processor (BPMP)
>>>> +
>>>> +The BPMP is a specific processor in Tegra chip, which is designed for
>>>> +booting process handling and offloading the power management, clock
>>>> +management, and reset control tasks from the CPU. The binding document
>>>> +defines the resources that would be used by the BPMP firmware driver,
>>>> +which can create the interprocessor communication (IPC) between the
>>>> CPU
>>>> +and BPMP.
>>>> +
>>>> +Required properties:
>>>> +- name : Should be bpmp
>>>> +- compatible
>>>> +    Array of strings
>>>> +    One of:
>>>> +    - "nvidia,tegra186-bpmp"
>>>> +- mboxes : The phandle of mailbox controller and the mailbox
>>>> specifier.
>>>> +- shmem : List of the phandle of the TX and RX shared memory area that
>>>> +      the IPC between CPU and BPMP is based on.
>>>
>>> I think you can use memory-region here.
>>
>> Isn't memory-region intended for references into the /reserved-memory
>> node. If so, that isn't appropriate in this case since this property
>> typically points at on-chip SRAM that isn't included in the OS's view of
>> "system RAM".
> Agree with that.
>
>>
>> Or, should /reserved-memory be used even for (e.g. non-DRAM) memory
>> regions that aren't represented by the /memory/reg property?
>>
>
> For shmem, I follow the same concept of the binding for arm,scpi
> (.../arm/arm,scpi.txt) that is currently using in mainline. Do you think
> that is more appropriate here?

Personally I think the shmem property name used by the current patch is 
fine. Still, if Rob feels strongly about changing it, that's fine too.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web