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

Groups > linux.kernel > #1712905 > unrolled thread

[PATCHv2 0/3] arm64 xilinx zynqmp firmware interface

Back to article view | Back to linux.kernel

Contents

  [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface Michal Simek <michal.simek@xilinx.com> - 2017-08-16 14:30 +0200
    [PATCHv2 2/3] arm64: zynqmp: dt: Add PM firmware node Michal Simek <michal.simek@xilinx.com> - 2017-08-16 14:30 +0200
    [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware Michal Simek <michal.simek@xilinx.com> - 2017-08-16 14:30 +0200
      Re: [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware Sudeep Holla <sudeep.holla@arm.com> - 2017-08-16 17:50 +0200
      Re: [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware Mark Rutland <mark.rutland@arm.com> - 2017-08-16 18:10 +0200
    Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface Marc Zyngier <marc.zyngier@arm.com> - 2017-08-16 17:50 +0200
      Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface Marc Zyngier <marc.zyngier@arm.com> - 2017-08-17 10:00 +0200
        Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface Sudeep Holla <sudeep.holla@arm.com> - 2017-08-17 11:20 +0200
          Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface Sudeep Holla <sudeep.holla@arm.com> - 2017-08-17 13:20 +0200

#1712905 — [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface

Hi,

xilinx is using this interface for very long time and we can't merge our
driver changes to Linux because of missing communication layer with
firmware. This interface was developed before scpi and scmi was
available. In connection to power management scpi and scmi are missing
pieces which we already use. There is a separate discussion how to
extend scmi to support all our use cases.
This simply patch is not adding any power management features but only
adding minimum functionality which are needed for drivers.

Edgar's comment on having HVC:
"We have patches for Xen that implement a power-management mediator.
That implementation handles PM calls over both SMC and HVC insns.
Patches are on the Xen mailing list.

Other hypervisors may work differently.

I think we should support both but I don't have a strong opinion on
it."

Thanks,
Michal

Changes in v2:
- Move to bindings/firmware and also add it to firmware node
  Reported-by Rob
- Add zynqmp firmware node to firmware node
- Change commit message
- Make zynqmp_pm_ret_code, invoke_pm_fn static
  Reported by Arnd
- Remove pm_api_version function - no need for interface
- Move driver to drivers/firmware/
  Reported by Arnd and Rob

Michal Simek (1):
  soc: xilinx: zynqmp: Add firmware interface

Soren Brinkmann (2):
  dt: xilinx: zynqmp: Add bindings for PM firmware
  arm64: zynqmp: dt: Add PM firmware node

 .../bindings/firmware/xlnx,zynqmp-pm.txt           |  22 ++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi             |   9 +
 drivers/firmware/Kconfig                           |   1 +
 drivers/firmware/Makefile                          |   1 +
 drivers/firmware/xilinx/Kconfig                    |   6 +
 drivers/firmware/xilinx/Makefile                   |   1 +
 drivers/firmware/xilinx/zynqmp/firmware.c          | 382 +++++++++++++++++++++
 include/linux/firmware/xilinx/zynqmp/firmware.h    | 232 +++++++++++++
 8 files changed, 654 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
 create mode 100644 drivers/firmware/xilinx/Kconfig
 create mode 100644 drivers/firmware/xilinx/Makefile
 create mode 100644 drivers/firmware/xilinx/zynqmp/firmware.c
 create mode 100644 include/linux/firmware/xilinx/zynqmp/firmware.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1712906 — [PATCHv2 2/3] arm64: zynqmp: dt: Add PM firmware node

From: Soren Brinkmann <soren.brinkmann@xilinx.com>

Add node for the firmware interface.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Add zynqmp firmware node to firmware node
- Change commit message

 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 54dc28351c8c..41c0034fd905 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -49,6 +49,15 @@
 		};
 	};
 
+	firmware {
+		zynqmp-firmware {
+			compatible = "xlnx,zynqmp-pm";
+			method = "smc";
+			interrupt-parent = <&gic>;
+			interrupts = <0 35 4>;
+		};
+	};
+
 	pmu {
 		compatible = "arm,armv8-pmuv3";
 		interrupt-parent = <&gic>;
-- 
1.9.1

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

#1712908 — [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware

From: Soren Brinkmann <soren.brinkmann@xilinx.com>

Document the DT bindings for the Zynq UltraScale+ PM Firmware.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Move to bindings/firmware and also add it to firmware node
  Reported-by Rob

 .../bindings/firmware/xlnx,zynqmp-pm.txt           | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt

diff --git a/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
new file mode 100644
index 000000000000..7de0c82758b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
@@ -0,0 +1,22 @@
+Xilinx Zynq MPSoC Firmware Device Tree Bindings
+
+The zynqmp-pm node describes the interface to platform firmware.
+
+Required properties:
+ - compatible:	Must contain:  "xlnx,zynqmp-pm"
+ - method:	The method of calling the PM-API firmware layer.
+		Permitted values are:
+		 - "smc" : To be used in configurations without a hypervisor
+		 - "hvc" : To be used when hypervisor is present
+ - interrupts:	Interrupt specifier
+
+Examples:
+
+	firmware {
+		zynqmp-firmware {
+			compatible = "xlnx,zynqmp-pm";
+			method = "smc";
+			interrupt-parent = <&gic>;
+			interrupts = <0 35 4>;
+		};
+	};
-- 
1.9.1

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

#1713071 — Re: [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware

On 16/08/17 13:24, Michal Simek wrote:
> From: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> Document the DT bindings for the Zynq UltraScale+ PM Firmware.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes in v2:
> - Move to bindings/firmware and also add it to firmware node
>   Reported-by Rob
> 
>  .../bindings/firmware/xlnx,zynqmp-pm.txt           | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> new file mode 100644
> index 000000000000..7de0c82758b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> @@ -0,0 +1,22 @@
> +Xilinx Zynq MPSoC Firmware Device Tree Bindings
> +
> +The zynqmp-pm node describes the interface to platform firmware.
> +
> +Required properties:
> + - compatible:	Must contain:  "xlnx,zynqmp-pm"
> + - method:	The method of calling the PM-API firmware layer.
> +		Permitted values are:
> +		 - "smc" : To be used in configurations without a hypervisor
> +		 - "hvc" : To be used when hypervisor is present
> + - interrupts:	Interrupt specifier

What exactly is this interrupt ? I see it's not SGI, so if it's SPI,
where does this originate from ? How is that dealt ?

-- 
Regards,
Sudeep

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

#1713082 — Re: [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware

On Wed, Aug 16, 2017 at 02:24:56PM +0200, Michal Simek wrote:
> From: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> Document the DT bindings for the Zynq UltraScale+ PM Firmware.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes in v2:
> - Move to bindings/firmware and also add it to firmware node
>   Reported-by Rob
> 
>  .../bindings/firmware/xlnx,zynqmp-pm.txt           | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> new file mode 100644
> index 000000000000..7de0c82758b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt
> @@ -0,0 +1,22 @@
> +Xilinx Zynq MPSoC Firmware Device Tree Bindings
> +
> +The zynqmp-pm node describes the interface to platform firmware.

Is there any documentation for this? I appreciate that the answer
might be "no".

> +Required properties:
> + - compatible:	Must contain:  "xlnx,zynqmp-pm"
> + - method:	The method of calling the PM-API firmware layer.
> +		Permitted values are:
> +		 - "smc" : To be used in configurations without a hypervisor
> +		 - "hvc" : To be used when hypervisor is present
> + - interrupts:	Interrupt specifier

For what, exactly?

Is this an interrupt for FW to signal the OS somehow?

If so, is it an SGI?

Thanks,
Mark.

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

#1713070

On 16/08/17 13:24, Michal Simek wrote:
> Hi,
> 
> xilinx is using this interface for very long time and we can't merge our
> driver changes to Linux because of missing communication layer with
> firmware. This interface was developed before scpi and scmi was
> available. In connection to power management scpi and scmi are missing
> pieces which we already use. There is a separate discussion how to
> extend scmi to support all our use cases.

So maybe we should wait and see where this discussion is going before we
merge yet another firmware interface?

> This simply patch is not adding any power management features but only
> adding minimum functionality which are needed for drivers.

I don't get it. 600 lines of firmware interface that isn't used by
anything? Or is it? Needed by which driver?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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

#1713639

On 17/08/17 07:10, Michal Simek wrote:
> On 16.8.2017 17:39, Marc Zyngier wrote:
>> On 16/08/17 13:24, Michal Simek wrote:
>>> Hi,
>>>
>>> xilinx is using this interface for very long time and we can't merge our
>>> driver changes to Linux because of missing communication layer with
>>> firmware. This interface was developed before scpi and scmi was
>>> available. In connection to power management scpi and scmi are missing
>>> pieces which we already use. There is a separate discussion how to
>>> extend scmi to support all our use cases.
>>
>> So maybe we should wait and see where this discussion is going before we
>> merge yet another firmware interface?
> 
> It will take a lot of time when this discussion ends and I can't see any
> benefit to hold all

Well, so far, the benefit of this series is exactly nil, as the code it
brings is *unused*. It is impossible to review in isolation.

In the meantime, you can continue finding out how *not* to have to merge
this code, and instead focus on using the infrastructure we already
have, or at least influence the infrastructure that is being designed.
It will be much better than dumping yet another slab of "I'm so
different" code that is going to ultimately bitrot.

> 
> 
>>> This simply patch is not adding any power management features but only
>>> adding minimum functionality which are needed for drivers.
>>
>> I don't get it. 600 lines of firmware interface that isn't used by
>> anything? Or is it? Needed by which driver?
> 
> I can send that drivers for review. pinctrl, fpga manager, nvmem driver,
> clock, serdes phy and reset drivers.
> But this driver need to come first because they depend on this.
My take is: no users, no use.

And if you need to hack all these drivers to hook into your specific
firmware interface, I feel that you're doing it wrong. We have common
APIs for device drivers. If these APIs are not good enough, we extend
them. But designing drivers for a given firmware interface?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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

#1713714

On 17/08/17 09:42, Michal Simek wrote:
> On 17.8.2017 09:52, Marc Zyngier wrote:
>> On 17/08/17 07:10, Michal Simek wrote:
>>> On 16.8.2017 17:39, Marc Zyngier wrote:
>>>> On 16/08/17 13:24, Michal Simek wrote:
>>>>> Hi,
>>>>>
>>>>> xilinx is using this interface for very long time and we can't merge our
>>>>> driver changes to Linux because of missing communication layer with
>>>>> firmware. This interface was developed before scpi and scmi was
>>>>> available. In connection to power management scpi and scmi are missing
>>>>> pieces which we already use. There is a separate discussion how to
>>>>> extend scmi to support all our use cases.
>>>>
>>>> So maybe we should wait and see where this discussion is going before we
>>>> merge yet another firmware interface?
>>>
>>> It will take a lot of time when this discussion ends and I can't see any
>>> benefit to hold all
>>
>> Well, so far, the benefit of this series is exactly nil, as the code it
>> brings is *unused*. It is impossible to review in isolation.
>>
> 
> Ok. I will add others drivers to this series that's not a problem.
> 
>> In the meantime, you can continue finding out how *not* to have to merge
>> this code, and instead focus on using the infrastructure we already
>> have, or at least influence the infrastructure that is being designed.
>> It will be much better than dumping yet another slab of "I'm so
>> different" code that is going to ultimately bitrot.
> 
> I am happy to look the better proposed way. SCPI is ancient and SCMI is
> replacement and not merged yet. We already had a call with arm and
> Sudeep was on it too where outcome from that was that we can't use that
> because it doesn't support what we need to support now.
> 

OK, none of the specifics were discussed in the meeting to conclude that
SCMI can't be used. My takeaway from the meeting was Xilinx has this
interface for long and being deployed in various systems. I would like
to get into specifics before discarding SCMI as unusable. What bothers
me more is that why was that not raised during the specification review
which was quite a long period IMO ? I tend to think Xilinx never
bothered to look/review the specification as this f/w interface was
already there.

However I still can't see why this was posted once we started pushing
out SCMI patches especially given that this f/w interface was there for
long and no attempts were made in past to upstream this.

Also I am not dismissing the series yet, but if I find that SCMI can be
used(after getting specifics from this series myself), I will at-least
argue against the "SCMI can't be used" argument.

-- 
Regards,
Sudeep

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

#1713827

On 17/08/17 11:32, Michal Simek wrote:
> On 17.8.2017 11:12, Sudeep Holla wrote:
>> 
>> 
>> On 17/08/17 09:42, Michal Simek wrote:
>>> On 17.8.2017 09:52, Marc Zyngier wrote:
>>>> On 17/08/17 07:10, Michal Simek wrote:
>>>>> On 16.8.2017 17:39, Marc Zyngier wrote:
>>>>>> On 16/08/17 13:24, Michal Simek wrote:
>>>>>>> Hi,
>>>>>>> 
>>>>>>> xilinx is using this interface for very long time and we
>>>>>>> can't merge our driver changes to Linux because of
>>>>>>> missing communication layer with firmware. This interface
>>>>>>> was developed before scpi and scmi was available. In
>>>>>>> connection to power management scpi and scmi are missing 
>>>>>>> pieces which we already use. There is a separate
>>>>>>> discussion how to extend scmi to support all our use
>>>>>>> cases.
>>>>>> 
>>>>>> So maybe we should wait and see where this discussion is
>>>>>> going before we merge yet another firmware interface?
>>>>> 
>>>>> It will take a lot of time when this discussion ends and I
>>>>> can't see any benefit to hold all
>>>> 
>>>> Well, so far, the benefit of this series is exactly nil, as the
>>>> code it brings is *unused*. It is impossible to review in
>>>> isolation.
>>>> 
>>> 
>>> Ok. I will add others drivers to this series that's not a
>>> problem.
>>> 
>>>> In the meantime, you can continue finding out how *not* to have
>>>> to merge this code, and instead focus on using the
>>>> infrastructure we already have, or at least influence the
>>>> infrastructure that is being designed. It will be much better
>>>> than dumping yet another slab of "I'm so different" code that
>>>> is going to ultimately bitrot.
>>> 
>>> I am happy to look the better proposed way. SCPI is ancient and
>>> SCMI is replacement and not merged yet. We already had a call
>>> with arm and Sudeep was on it too where outcome from that was
>>> that we can't use that because it doesn't support what we need to
>>> support now.
>>> 
>> 
>> OK, none of the specifics were discussed in the meeting to conclude
>> that SCMI can't be used. My takeaway from the meeting was Xilinx
>> has this interface for long and being deployed in various systems.
>> I would like to get into specifics before discarding SCMI as
>> unusable. What bothers me more is that why was that not raised
>> during the specification review which was quite a long period IMO ?
>> I tend to think Xilinx never bothered to look/review the
>> specification as this f/w interface was already there.
> 
> Xilinx is using this interface from Aug 2015. I am not aware about
> any invitation to spec review. And not sure who was there from
> xilinx.
> 

Sure, I can understand and that's not a problem but Xilinx was involved.

>> 
>> However I still can't see why this was posted once we started
>> pushing out SCMI patches especially given that this f/w interface
>> was there for long and no attempts were made in past to upstream
>> this.
> 
> The reason is simple which is upstream our code which depends on
> this communication layer. I don't think there is quick path to move
> to different interface than this one.
> 

Do you mean "smc" when you refer communication layer ? If so, that's
fine. You can use "smc" as transport with SCMI if you want,
specification doesn't prevent that.

>> 
>> Also I am not dismissing the series yet, but if I find that SCMI
>> can be used(after getting specifics from this series myself), I
>> will at-least argue against the "SCMI can't be used" argument.
> 
> This is not my argument that we can't use SCMI. This is what was my 
> understanding from that meeting we had. And definitely there is no
> quick path for us to switch to SCMI and breaks all current existing
> customers.
> 

I understand the latter and I mentioned the same earlier, but I disagree
with the former. That meeting was mostly introduction(and informal IMO)
and didn't involve anything at the technical level.

> And this interface is just in the same position as current SCPI. It 
> means you have SCPI already merged and you are adding new one. SCMI 
> could be maybe also just SCPIv2.

Agreed, but it was posted as soon as the specification is out and so is
the SCMI. I am not arguing it as a point, but just mentioning that this
post was simply bad timing :)

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web