diff mbox series

[U-Boot,v2,04/13] firmware: zynqmp: Add zynqmp-power support

Message ID 6186f16b7874c6ebdd419b03f101bd574c178adf.1570023563.git.michal.simek@xilinx.com
State New
Delegated to: Michal Simek
Headers show
Series arm64: zynqmp: Clean communication with PMUFW | expand

Commit Message

Michal Simek Oct. 2, 2019, 1:39 p.m. UTC
From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

zynqmp-power driver for ZynqMP to handle the communication with the PMU
firmware. Firmware driver just probes subnodes and power driver handles
communication with PMU using the IPI mailbox driver.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Check error separately - Reported by Luca

 drivers/firmware/Kconfig           |  2 ++
 drivers/firmware/firmware-zynqmp.c | 44 ++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

Comments

Luca Ceresoli Oct. 9, 2019, 3:03 p.m. UTC | #1
Hi,

On 02/10/19 15:39, Michal Simek wrote:
> From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> 
> zynqmp-power driver for ZynqMP to handle the communication with the PMU
> firmware. Firmware driver just probes subnodes and power driver handles
> communication with PMU using the IPI mailbox driver.
> 
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes in v2:
> - Check error separately - Reported by Luca
> 
>  drivers/firmware/Kconfig           |  2 ++
>  drivers/firmware/firmware-zynqmp.c | 44 ++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b70a2063551c..9596ec16c7f7 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL
>  config ZYNQMP_FIRMWARE
>  	bool "ZynqMP Firmware interface"
>  	select FIRMWARE
> +	select ZYNQMP_IPI
> +	select DM_MAILBOX

Nitpick: ZYNQMP_IPI depends on DM_MAILBOX, so I'd list them in opposite
order. Just for clarity.

>  	help
>  	  Firmware interface driver is used by different
>  	  drivers to communicate with the firmware for
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index 6644a7166ca0..97ac333296ec 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -1,6 +1,50 @@
>  // SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx Zynq MPSoC Firmware driver
> + *
> + * Copyright (C) 2018-2019 Xilinx, Inc.
> + */
>  
> +#include <common.h>
>  #include <dm.h>
> +#include <mailbox.h>
> +#include <asm/arch/sys_proto.h>
> +
> +struct zynqmp_power {
> +	struct mbox_chan tx_chan;
> +	struct mbox_chan rx_chan;
> +} zynqmp_power;
> +
> +static int zynqmp_power_probe(struct udevice *dev)
> +{
> +	int ret = 0;

No need to initialize.

> +
> +	debug("%s, (dev=%p)\n", __func__, dev);
> +
> +	ret = mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
> +	if (ret) {
> +		debug("%s, cannot tx mailbox\n", __func__);

"cannot tx" -> "cannot find tx"?

> +		return ret;
> +	}
> +
> +	ret = mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
> +	if (ret)
> +		debug("%s, cannot rx mailbox\n", __func__);

Ditto.

> +
> +	return ret;

return 0;

PS: I think some of the above are fixed in later patches, but some are
still valid.
Michal Simek Oct. 10, 2019, 9:24 a.m. UTC | #2
On 09. 10. 19 17:03, Luca Ceresoli wrote:
> Hi,
> 
> On 02/10/19 15:39, Michal Simek wrote:
>> From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>
>> zynqmp-power driver for ZynqMP to handle the communication with the PMU
>> firmware. Firmware driver just probes subnodes and power driver handles
>> communication with PMU using the IPI mailbox driver.
>>
>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Changes in v2:
>> - Check error separately - Reported by Luca
>>
>>  drivers/firmware/Kconfig           |  2 ++
>>  drivers/firmware/firmware-zynqmp.c | 44 ++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index b70a2063551c..9596ec16c7f7 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL
>>  config ZYNQMP_FIRMWARE
>>  	bool "ZynqMP Firmware interface"
>>  	select FIRMWARE
>> +	select ZYNQMP_IPI
>> +	select DM_MAILBOX
> 
> Nitpick: ZYNQMP_IPI depends on DM_MAILBOX, so I'd list them in opposite
> order. Just for clarity.
> 

This has changed already.

>>  	help
>>  	  Firmware interface driver is used by different
>>  	  drivers to communicate with the firmware for
>> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
>> index 6644a7166ca0..97ac333296ec 100644
>> --- a/drivers/firmware/firmware-zynqmp.c
>> +++ b/drivers/firmware/firmware-zynqmp.c
>> @@ -1,6 +1,50 @@
>>  // SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx Zynq MPSoC Firmware driver
>> + *
>> + * Copyright (C) 2018-2019 Xilinx, Inc.
>> + */
>>  
>> +#include <common.h>
>>  #include <dm.h>
>> +#include <mailbox.h>
>> +#include <asm/arch/sys_proto.h>
>> +
>> +struct zynqmp_power {
>> +	struct mbox_chan tx_chan;
>> +	struct mbox_chan rx_chan;
>> +} zynqmp_power;
>> +
>> +static int zynqmp_power_probe(struct udevice *dev)
>> +{
>> +	int ret = 0;
> 
> No need to initialize.

Will fix.

> 
>> +
>> +	debug("%s, (dev=%p)\n", __func__, dev);
>> +
>> +	ret = mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
>> +	if (ret) {
>> +		debug("%s, cannot tx mailbox\n", __func__);
> 
> "cannot tx" -> "cannot find tx"?

ditto.

> 
>> +		return ret;
>> +	}
>> +
>> +	ret = mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
>> +	if (ret)
>> +		debug("%s, cannot rx mailbox\n", __func__);
> 
> Ditto.

ditto.

> 
>> +
>> +	return ret;
> 
> return 0;

return ret was correct here because if (ret) doesn't return error value
and that code just print message in case of error.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b70a2063551c..9596ec16c7f7 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -30,6 +30,8 @@  config TI_SCI_PROTOCOL
 config ZYNQMP_FIRMWARE
 	bool "ZynqMP Firmware interface"
 	select FIRMWARE
+	select ZYNQMP_IPI
+	select DM_MAILBOX
 	help
 	  Firmware interface driver is used by different
 	  drivers to communicate with the firmware for
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index 6644a7166ca0..97ac333296ec 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -1,6 +1,50 @@ 
 // SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Zynq MPSoC Firmware driver
+ *
+ * Copyright (C) 2018-2019 Xilinx, Inc.
+ */
 
+#include <common.h>
 #include <dm.h>
+#include <mailbox.h>
+#include <asm/arch/sys_proto.h>
+
+struct zynqmp_power {
+	struct mbox_chan tx_chan;
+	struct mbox_chan rx_chan;
+} zynqmp_power;
+
+static int zynqmp_power_probe(struct udevice *dev)
+{
+	int ret = 0;
+
+	debug("%s, (dev=%p)\n", __func__, dev);
+
+	ret = mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
+	if (ret) {
+		debug("%s, cannot tx mailbox\n", __func__);
+		return ret;
+	}
+
+	ret = mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
+	if (ret)
+		debug("%s, cannot rx mailbox\n", __func__);
+
+	return ret;
+};
+
+static const struct udevice_id zynqmp_power_ids[] = {
+	{ .compatible = "xlnx,zynqmp-power" },
+	{ }
+};
+
+U_BOOT_DRIVER(zynqmp_power) = {
+	.name = "zynqmp_power",
+	.id = UCLASS_FIRMWARE,
+	.of_match = zynqmp_power_ids,
+	.probe = zynqmp_power_probe,
+};
 
 static const struct udevice_id zynqmp_firmware_ids[] = {
 	{ .compatible = "xlnx,zynqmp-firmware" },