Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.60.100 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.60.100; helo=xsj-pvapsmtpgw02;
Subject: Re: [PATCH] drivers: clk: Update clock driver to handle clock
 attribute
To:     Jolly Shah <jolly.shah@xilinx.com>, <mturquette@baylibre.com>,
        <sboyd@codeaurora.org>, <michal.simek@xilinx.com>,
        <linux-clk@vger.kernel.org>
CC:     <rajanv@xilinx.com>, <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, Rajan Vaja <rajan.vaja@xilinx.com>,
        Tejas Patel <tejas.patel@xilinx.com>,
        Jolly Shah <jollys@xilinx.com>
References: <1551741550-10315-1-git-send-email-jollys@xilinx.com>
From:   Michal Simek <michal.simek@xilinx.com>
Message-ID: <f09ba7a2-6379-cebe-e803-b7b67686fa6e@xilinx.com>
Date:   Mon, 18 Mar 2019 13:48:17 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <1551741550-10315-1-git-send-email-jollys@xilinx.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Mar 2019 12:48:33.1923
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 96ceec8c-99e0-4c31-ce26-08d6aba007d9
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.100];Helo=[xsj-pvapsmtpgw02]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR02MB4054
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 05. 03. 19 0:19, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Versal EEMI APIs uses clock device ID which is combination of class,
> subclass, type and clock index (e.g. 0x8104006 in which 0-13 bits are
> for index(6 in given example), 14-19 bits are for clock type (i.e pll,
> out or ref, 1 in given example), 20-25 bits are for subclass which is
> nothing but clock type only), 26-32 bits are for device class, which
> is clock(0x2) for all clocks) while zynqmp firmware uses clock ID
> which is index only (e.g 0, 1, to n, where n is max_clock id).
> 
> To use zynqmp clock driver for versal platform also, extend use
> of QueryAttribute API to fetch device class, subclass and clock type
> to create clock device ID. In case of zynqmp this attributes would be
> 0 only, so there won't be any effect on clock id as it would use
> clock index only.
> 
> Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Jolly Shah <jollys@xilinx.com>
> ---
>  drivers/clk/zynqmp/clkc.c | 42 +++++++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> index f65cc0f..c13b014 100644
> --- a/drivers/clk/zynqmp/clkc.c
> +++ b/drivers/clk/zynqmp/clkc.c
> @@ -53,6 +53,10 @@
>  #define RESERVED_CLK_NAME		""
>  
>  #define CLK_VALID_MASK			0x1
> +#define NODE_CLASS_SHIFT		26U
> +#define NODE_SUBCLASS_SHIFT		20U
> +#define NODE_TYPE_SHIFT			14U
> +#define NODE_INDEX_SHIFT		0U
>  
>  enum clk_type {
>  	CLK_TYPE_OUTPUT,
> @@ -80,6 +84,7 @@ struct clock_parent {
>   * @num_nodes:		Number of nodes present in topology
>   * @parent:		Parent of clock
>   * @num_parents:	Number of parents of clock
> + * @clk_id:		Clock id
>   */
>  struct zynqmp_clock {
>  	char clk_name[MAX_NAME_LEN];
> @@ -89,6 +94,7 @@ struct zynqmp_clock {
>  	u32 num_nodes;
>  	struct clock_parent parent[MAX_PARENT];
>  	u32 num_parents;
> +	u32 clk_id;
>  };
>  
>  static const char clk_type_postfix[][10] = {
> @@ -396,7 +402,8 @@ static int zynqmp_clock_get_topology(u32 clk_id,
>  
>  	*num_nodes = 0;
>  	for (j = 0; j <= MAX_NODES; j += 3) {
> -		ret = zynqmp_pm_clock_get_topology(clk_id, j, pm_resp);
> +		ret = zynqmp_pm_clock_get_topology(clock[clk_id].clk_id, j,
> +						   pm_resp);
>  		if (ret)
>  			return ret;
>  		ret = __zynqmp_clock_get_topology(topology, pm_resp, num_nodes);
> @@ -459,7 +466,8 @@ static int zynqmp_clock_get_parents(u32 clk_id, struct clock_parent *parents,
>  	*num_parents = 0;
>  	do {
>  		/* Get parents from firmware */
> -		ret = zynqmp_pm_clock_get_parents(clk_id, j, pm_resp);
> +		ret = zynqmp_pm_clock_get_parents(clock[clk_id].clk_id, j,
> +						  pm_resp);
>  		if (ret)
>  			return ret;
>  
> @@ -528,13 +536,14 @@ static struct clk_hw *zynqmp_register_clk_topology(int clk_id, char *clk_name,
>  						   const char **parent_names)
>  {
>  	int j;
> -	u32 num_nodes;
> +	u32 num_nodes, clk_dev_id;
>  	char *clk_out = NULL;
>  	struct clock_topology *nodes;
>  	struct clk_hw *hw = NULL;
>  
>  	nodes = clock[clk_id].node;
>  	num_nodes = clock[clk_id].num_nodes;
> +	clk_dev_id = clock[clk_id].clk_id;
>  
>  	for (j = 0; j < num_nodes; j++) {
>  		/*
> @@ -551,13 +560,14 @@ static struct clk_hw *zynqmp_register_clk_topology(int clk_id, char *clk_name,
>  		if (!clk_topology[nodes[j].type])
>  			continue;
>  
> -		hw = (*clk_topology[nodes[j].type])(clk_out, clk_id,
> +		hw = (*clk_topology[nodes[j].type])(clk_out, clk_dev_id,
>  						    parent_names,
>  						    num_parents,
>  						    &nodes[j]);
>  		if (IS_ERR(hw))
> -			pr_warn_once("%s() %s register fail with %ld\n",
> -				     __func__, clk_name, PTR_ERR(hw));
> +			pr_warn_once("%s() 0x%x: %s register fail with %ld\n",
> +				     __func__,  clk_dev_id, clk_name,
> +				     PTR_ERR(hw));
>  
>  		parent_names[0] = clk_out;
>  	}
> @@ -621,20 +631,26 @@ static int zynqmp_register_clocks(struct device_node *np)
>  static void zynqmp_get_clock_info(void)
>  {
>  	int i, ret;
> -	u32 attr, type = 0;
> +	u32 attr, type = 0, nodetype, subclass, class;
>  
>  	for (i = 0; i < clock_max_idx; i++) {
> -		zynqmp_pm_clock_get_name(i, clock[i].clk_name);
> -		if (!strcmp(clock[i].clk_name, RESERVED_CLK_NAME))
> -			continue;
> -
>  		ret = zynqmp_pm_clock_get_attributes(i, &attr);
>  		if (ret)
>  			continue;
>  
>  		clock[i].valid = attr & CLK_VALID_MASK;
> -		clock[i].type = attr >> CLK_TYPE_SHIFT ? CLK_TYPE_EXTERNAL :
> -							CLK_TYPE_OUTPUT;
> +		clock[i].type = ((attr >> CLK_TYPE_SHIFT) & 0x1) ?
> +				CLK_TYPE_EXTERNAL : CLK_TYPE_OUTPUT;
> +		nodetype = (attr >> NODE_TYPE_SHIFT) & 0x3F;
> +		subclass = (attr >> NODE_SUBCLASS_SHIFT) & 0x3F;
> +		class = (attr >> NODE_CLASS_SHIFT) & 0x3F;
> +
> +		clock[i].clk_id = (class << NODE_CLASS_SHIFT) |
> +				  (subclass << NODE_SUBCLASS_SHIFT) |
> +				  (nodetype << NODE_TYPE_SHIFT) |
> +				  (i << NODE_INDEX_SHIFT);
> +
> +		zynqmp_pm_clock_get_name(clock[i].clk_id, clock[i].clk_name);
>  	}
>  
>  	/* Get topology of all clock */
> 

Stephen: Do you want to take this via your tree?

Thanks,
Michal