Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.60.83 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.60.83; helo=xsj-pvapsmtpgw01;
Subject: Re: [PATCH v2 2/2] drivers: clk: zynqmp: Update fraction clock check
 from custom type flags
To:     Stephen Boyd <sboyd@kernel.org>,
        Jolly Shah <jolly.shah@xilinx.com>, arm@kernel.org,
        linux-clk@vger.kernel.org, michal.simek@xilinx.com,
        mturquette@baylibre.com, olof@lixom.net
Cc:     rajanv@xilinx.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, Tejas Patel <tejas.patel@xilinx.com>,
        Rajan Vaja <rajan.vaja@xilinx.com>
References: <1584048699-24186-1-git-send-email-jolly.shah@xilinx.com>
 <1584048699-24186-3-git-send-email-jolly.shah@xilinx.com>
 <159054169658.88029.371843532116000844@swboyd.mtv.corp.google.com>
 <2c8cd31a-46ba-ec6a-67a7-f3d9abe561ff@xilinx.com>
 <159070755756.69627.18401650656284023600@swboyd.mtv.corp.google.com>
From:   Jolly Shah <jolly.shah@xilinx.com>
Message-ID: <725e9477-647f-83b7-7fdf-7cdb7ae74586@xilinx.com>
Date:   Fri, 29 May 2020 14:20:55 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
 Gecko/20100101 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <159070755756.69627.18401650656284023600@swboyd.mtv.corp.google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2020 21:21:07.9260
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 7e236307-d780-4db1-9398-08d8041633e5
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.83];Helo=[xsj-pvapsmtpgw01]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR02MB2196
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Stephan,

 > ------Original Message------
 > From: Stephen Boyd <sboyd@kernel.org>
 > Sent:  Thursday, May 28, 2020 4:12PM
 > To: Jolly Shah <jolly.shah@xilinx.com>, Arm <arm@kernel.org>, 
Linux-clk <linux-clk@vger.kernel.org>, Michal Simek 
<michal.simek@xilinx.com>, Mturquette <mturquette@baylibre.com>, Olof 
<olof@lixom.net>
 > Cc: Rajan Vaja <rajanv@xilinx.com>, 
Linux-arm-kernel@lists.infradead.org 
<linux-arm-kernel@lists.infradead.org>, Linux-kernel@vger.kernel.org 
<linux-kernel@vger.kernel.org>, Tejas Patel <tejas.patel@xilinx.com>, 
Rajan Vaja <rajan.vaja@xilinx.com>
 > Subject: Re: [PATCH v2 2/2] drivers: clk: zynqmp: Update fraction 
clock check from custom type flags
 >
> Quoting Jolly Shah (2020-05-28 10:44:01)
>> Hi Stephan,
>>
>> Thanks for the review.
>>
>>   > ------Original Message------
>>   > From: Stephen Boyd <sboyd@kernel.org>
>>   > Sent:  Tuesday, May 26, 2020 6:08PM
>>   > To: Jolly Shah <jolly.shah@xilinx.com>, Arm <arm@kernel.org>,
>> Linux-clk <linux-clk@vger.kernel.org>, Michal Simek
>> <michal.simek@xilinx.com>, Mturquette <mturquette@baylibre.com>, Olof
>> <olof@lixom.net>
>>   > Cc: Rajan Vaja <rajanv@xilinx.com>,
>> Linux-arm-kernel@lists.infradead.org
>> <linux-arm-kernel@lists.infradead.org>, Linux-kernel@vger.kernel.org
>> <linux-kernel@vger.kernel.org>, Tejas Patel <tejas.patel@xilinx.com>,
>> Rajan Vaja <rajan.vaja@xilinx.com>, Jolly Shah <jolly.shah@xilinx.com>
>>   > Subject: Re: [PATCH v2 2/2] drivers: clk: zynqmp: Update fraction
>> clock check from custom type flags
>>   >
>>> Quoting Jolly Shah (2020-03-12 14:31:39)
>>>> From: Tejas Patel <tejas.patel@xilinx.com>
>>>>
>>>> Older firmware version sets BIT(13) in clkflag to mark a
>>>> divider as fractional divider. Updated firmware version sets BIT(4)
>>>> in type flags to mark a divider as fractional divider since
>>>> BIT(13) is defined as CLK_DUTY_CYCLE_PARENT in the common clk
>>>> framework flags.
>>>>
>>>> To support both old and new firmware version, consider BIT(13) from
>>>> clkflag and BIT(4) from type_flag to check if divider is fractional
>>>> or not.
>>>>
>>>> To maintain compatibility BIT(13) of clkflag in firmware will not be
>>>> used in future for any purpose and will be marked as unused.
>>>
>>> Why are we mixing the firmware flags with the ccf flags? They shouldn't
>>> be the same. The firmware should have its own 'flag numberspace' that is
>>> distinct from the common clk framework's 'flag numberspace'. Please fix
>>> the code.
>>>
>>
>> Yes firmware flags are using separate numberspace now. Firmware
>> maintains CCF and firmware specific flags separately but earlier
>> CLK_FRAC was mistakenly defined in ccf flagspace and hence handled here
>> for backward compatibility. Driver takes care of not registering same
>> with CCF. Let me know if I misunderstood.
> 
> Why is the firmware maintaining CCF specific flags? The firmware
> shouldn't know about the CCF flag numbering at all. We can change the
> numbers that the CCF flags are assigned to randomly and that shouldn't
> mean that the firmware needs to change. Maybe I should apply this patch?

Got it. Will fix it.

Thanks,
Jolly Shah


> 
> ---8<----
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index bd1ee9039558..c1f36bca85b0 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -16,22 +16,22 @@
>    *
>    * Please update clk_flags[] in drivers/clk/clk.c when making changes here!
>    */
> -#define CLK_SET_RATE_GATE	BIT(0) /* must be gated across rate change */
> -#define CLK_SET_PARENT_GATE	BIT(1) /* must be gated across re-parent */
> -#define CLK_SET_RATE_PARENT	BIT(2) /* propagate rate change up one level */
> -#define CLK_IGNORE_UNUSED	BIT(3) /* do not gate even if unused */
> +#define CLK_SET_RATE_GATE	BIT(13) /* must be gated across rate change */
> +#define CLK_SET_PARENT_GATE	BIT(2) /* must be gated across re-parent */
> +#define CLK_SET_RATE_PARENT	BIT(3) /* propagate rate change up one level */
> +#define CLK_IGNORE_UNUSED	BIT(4) /* do not gate even if unused */
>   				/* unused */
>   				/* unused */
> -#define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
> -#define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> -#define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> -#define CLK_RECALC_NEW_RATES	BIT(9) /* recalc rates after notifications */
> -#define CLK_SET_RATE_UNGATE	BIT(10) /* clock needs to run to set rate */
> -#define CLK_IS_CRITICAL		BIT(11) /* do not gate, ever */
> +#define CLK_GET_RATE_NOCACHE	BIT(5) /* do not use the cached clk rate */
> +#define CLK_SET_RATE_NO_REPARENT BIT(6) /* don't re-parent on rate change */
> +#define CLK_GET_ACCURACY_NOCACHE BIT(7) /* do not use the cached clk accuracy */
> +#define CLK_RECALC_NEW_RATES	BIT(8) /* recalc rates after notifications */
> +#define CLK_SET_RATE_UNGATE	BIT(9) /* clock needs to run to set rate */
> +#define CLK_IS_CRITICAL		BIT(10) /* do not gate, ever */
>   /* parents need enable during gate/ungate, set rate and re-parent */
> -#define CLK_OPS_PARENT_ENABLE	BIT(12)
> +#define CLK_OPS_PARENT_ENABLE	BIT(11)
>   /* duty cycle call may be forwarded to the parent clock */
> -#define CLK_DUTY_CYCLE_PARENT	BIT(13)
> +#define CLK_DUTY_CYCLE_PARENT	BIT(12)
>   
>   struct clk;
>   struct clk_hw;
>