Re: [PATCH v2 2/2] drivers: clk: zynqmp: Update fraction clock check from custom type flags

[Jolly Shah]

Hi Stephan,

Thanks for the review.

> ------Original Message------
> From: Stephen Boyd <sboyd@xxxxxxxxxx>
> Sent:  Tuesday, May 26, 2020 6:08PM
> To: Jolly Shah <jolly.shah@xxxxxxxxxx>, Arm <arm@xxxxxxxxxx>, 
Linux-clk <linux-clk@xxxxxxxxxxxxxxx>, Michal Simek 
<michal.simek@xxxxxxxxxx>, Mturquette <mturquette@xxxxxxxxxxxx>, Olof 
<olof@xxxxxxxxx>
> Cc: Rajan Vaja <rajanv@xxxxxxxxxx>, 
Linux-arm-kernel@xxxxxxxxxxxxxxxxxxx 
<linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>, Linux-kernel@xxxxxxxxxxxxxxx 
<linux-kernel@xxxxxxxxxxxxxxx>, Tejas Patel <tejas.patel@xxxxxxxxxx>, 
Rajan Vaja <rajan.vaja@xxxxxxxxxx>, Jolly Shah <jolly.shah@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> >
> > 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.

Thanks,
Jolly Shah