Messages in this thread | | | Subject | Re: [PATCH v2 2/2] drivers: clk: zynqmp: Update fraction clock check from custom type flags | From | Jolly Shah <> | Date | Fri, 29 May 2020 14:20:55 -0700 |
| |
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; >
| |