[PATCH v2] power: zynqmp: Add power domain driver for ZynqMP

[Michal Simek]

On 1/27/22 06:02, Jaehoon Chung wrote:
> On 1/20/22 16:25, Michal Simek wrote:
>> Driver should be enabled by CONFIG_POWER_DOMAIN=y and
>> CONFIG_ZYNQMP_POWER_DOMAIN=y. Power domain driver doesn't have own DT node
>> but it uses zynqmp firmware DT node that's why there is a need to bind
>> driver when firmware node is found.
>>
>> Driver itself is simple. It is sending pmufw config object overlay for
>> enabling access to device which is done in ...domain_request().
>> In ...domain_on() capabilities are passed and node is requested.
>> This should be bare minimum of required to get power domain driver working.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>> Changes in v2:
>> - s/binded/bound/ - Reported by gvb
>>
>>   MAINTAINERS                                |  1 +
>>   drivers/firmware/firmware-zynqmp.c         | 18 +++++
>>   drivers/power/domain/Kconfig               |  9 +++
>>   drivers/power/domain/Makefile              |  1 +
>>   drivers/power/domain/zynqmp-power-domain.c | 91 ++++++++++++++++++++++
>>   include/zynqmp_firmware.h                  | 31 ++++++++
>>   6 files changed, 151 insertions(+)
>>   create mode 100644 drivers/power/domain/zynqmp-power-domain.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 64648c29219f..fca5227d0b93 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -628,6 +628,7 @@ F:	drivers/mtd/nand/raw/zynq_nand.c
>>   F:	drivers/net/phy/xilinx_phy.c
>>   F:	drivers/net/zynq_gem.c
>>   F:	drivers/phy/phy-zynqmp.c
>> +F:	drivers/power/domain/zynqmp-power-domain.c
>>   F:	drivers/serial/serial_zynq.c
>>   F:	drivers/reset/reset-zynqmp.c
>>   F:	drivers/rtc/zynqmp_rtc.c
>> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
>> index 839203ec82a1..5a618e490725 100644
>> --- a/drivers/firmware/firmware-zynqmp.c
>> +++ b/drivers/firmware/firmware-zynqmp.c
>> @@ -8,6 +8,7 @@
>>   #include <common.h>
>>   #include <cpu_func.h>
>>   #include <dm.h>
>> +#include <dm/lists.h>
>>   #include <log.h>
>>   #include <zynqmp_firmware.h>
>>   #include <asm/cache.h>
>> @@ -226,8 +227,25 @@ static const struct udevice_id zynqmp_firmware_ids[] = {
>>   	{ }
>>   };
>>   
>> +static int zynqmp_firmware_bind(struct udevice *dev)
>> +{
>> +	int ret;
>> +	struct udevice *child;
>> +
>> +	if (IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) {
>> +		ret = device_bind_driver_to_node(dev, "zynqmp_power_domain",
>> +						 "zynqmp_power_domain",
>> +						 dev_ofnode(dev), &child);
>> +		if (ret)
>> +			printf("zynqmp power domain driver is not bound\n");
> 
> Doesn't need to handle error case? And how about displaying an error value(ret)?

I have added it there in v3

> 
>> +	}
>> +
>> +	return dm_scan_fdt_dev(dev);
>> +}
>> +
>>   U_BOOT_DRIVER(zynqmp_firmware) = {
>>   	.id = UCLASS_FIRMWARE,
>>   	.name = "zynqmp_firmware",
>>   	.of_match = zynqmp_firmware_ids,
>> +	.bind = zynqmp_firmware_bind,
>>   };
>> diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
>> index 9aea5fcdf082..93d2599d83c2 100644
>> --- a/drivers/power/domain/Kconfig
>> +++ b/drivers/power/domain/Kconfig
>> @@ -88,4 +88,13 @@ config TI_POWER_DOMAIN
>>   	help
>>   	  Generic power domain implementation for TI K3 devices.
>>   
>> +config ZYNQMP_POWER_DOMAIN
>> +	bool "Enable the Xilinx ZynqMP Power domain driver"
>> +	depends on POWER_DOMAIN && ZYNQMP_FIRMWARE
>> +	help
>> +	  Generic power domain implementation for Xilinx ZynqMP devices.
>> +	  The driver should be enabled when system starts in very minimal
>> +	  configuration and it is extended at run time. Then enabling
>> +	  the driver will ensure that PMUFW enable access to requested IP.
>> +
>>   endmenu
>> diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile
>> index 530ae35671a2..7c8af67dbd64 100644
>> --- a/drivers/power/domain/Makefile
>> +++ b/drivers/power/domain/Makefile
>> @@ -16,3 +16,4 @@ obj-$(CONFIG_SANDBOX_POWER_DOMAIN) += sandbox-power-domain-test.o
>>   obj-$(CONFIG_TEGRA186_POWER_DOMAIN) += tegra186-power-domain.o
>>   obj-$(CONFIG_TI_SCI_POWER_DOMAIN) += ti-sci-power-domain.o
>>   obj-$(CONFIG_TI_POWER_DOMAIN) += ti-power-domain.o
>> +obj-$(CONFIG_ZYNQMP_POWER_DOMAIN) += zynqmp-power-domain.o
>> diff --git a/drivers/power/domain/zynqmp-power-domain.c b/drivers/power/domain/zynqmp-power-domain.c
>> new file mode 100644
>> index 000000000000..0a6cb67a8acc
>> --- /dev/null
>> +++ b/drivers/power/domain/zynqmp-power-domain.c
>> @@ -0,0 +1,91 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2021, Xilinx. Inc.
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <log.h>
>> +#include <malloc.h>
>> +#include <misc.h>
>> +#include <power-domain-uclass.h>
>> +#include <linux/bitops.h>
>> +
>> +#include <zynqmp_firmware.h>
>> +
>> +#define NODE_ID_LOCATION	5
>> +
>> +static unsigned int xpm_configobject[] = {
>> +	/**********************************************************************/
> 
> Remove "/***....***/". It doesn't need to add.
> 
>> +	/* HEADER */
>> +	2,	/* Number of remaining words in the header */
>> +	1,	/* Number of sections included in config object */
>> +	PM_CONFIG_OBJECT_TYPE_OVERLAY,	/* Type of Config object as overlay*/
>> +	/**********************************************************************/
> 
> Ditto.

Both fixed.

Thanks,
Michal