Messages in this thread Patch in this message | | | From | "Ardelean, Alexandru" <> | Subject | Re: [PATCH] clk: clk-gpio: Add dt option to propagate rate change to parent | Date | Fri, 8 Nov 2019 07:10:29 +0000 |
| |
On Fri, 2019-11-08 at 06:50 +0000, Ardelean, Alexandru wrote: > [External] > > On Thu, 2019-11-07 at 14:53 -0800, Stephen Boyd wrote: > > Quoting Ardelean, Alexandru (2019-11-07 05:25:38) > > > On Wed, 2019-11-06 at 14:43 -0800, Stephen Boyd wrote: > > > > Quoting Alexandru Ardelean (2019-11-06 03:35:51) > > > > > From: Michael Hennerich <michael.hennerich@analog.com> > > > > > > > > > > For certain setups/boards it's useful to propagate the rate > > > > > change > > > > > of > > > > > the > > > > > clock up one level to the parent clock. > > > > > > > > > > This change implements this by defining a `clk-set-rate-parent` > > > > > device- > > > > > tree > > > > > property which sets the `CLK_SET_RATE_PARENT` flag to the clock > > > > > (when > > > > > set). > > > > > > > > > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > > > --- > > > > > drivers/clk/clk-gpio.c | 8 ++++++-- > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c > > > > > index 9d930edd6516..6dfbc4b952fe 100644 > > > > > --- a/drivers/clk/clk-gpio.c > > > > > +++ b/drivers/clk/clk-gpio.c > > > > > @@ -241,6 +241,7 @@ static int gpio_clk_driver_probe(struct > > > > > platform_device *pdev) > > > > > struct device_node *node = pdev->dev.of_node; > > > > > const char **parent_names, *gpio_name; > > > > > unsigned int num_parents; > > > > > + unsigned long clk_flags; > > > > > struct gpio_desc *gpiod; > > > > > struct clk *clk; > > > > > bool is_mux; > > > > > @@ -274,13 +275,16 @@ static int gpio_clk_driver_probe(struct > > > > > platform_device *pdev) > > > > > return ret; > > > > > } > > > > > > > > > > + clk_flags = of_property_read_bool(node, "clk-set-rate- > > > > > parent") > > > > > ? > > > > > + CLK_SET_RATE_PARENT : 0; > > > > > > > > Is there a DT binding update somewhere? It looks like a linux-ism > > > > from > > > > > > Good point. I did not think about the DT, and I guess I didn't search > > > it > > > thoroughly enough. Found DT files now. > > > > > > > the DT perspective. I wonder if we can somehow figure out that it's > > > > OK > > > > to call clk_set_rate() on the parent here? Or is it safe to assume > > > > that > > > > we can just always call set rate on the parent? I think for a gate > > > > it's > > > > good and we can just do so, but for a mux maybe not. Care to > > > > describe > > > > your scenario a little more so we can understand why you want to > > > > set > > > > this flag? Is it for a mux or a gate type gpio? > > > > > > > > > > For our case we are using it here [with a slight name variation in > > > the > > > prop > > > name]: > > > https://github.com/analogdevicesinc/linux/blob/master/arch/arm/boot/dts/zynq-adrv9361-z7035.dtsi#L43 > > > > > > And on this board: > > > https://wiki.analog.com/resources/eval/user-guides/adrv936x_rfsom/user-guide/introduction > > > > > > It's for a gate-type GPIO. > > > The clock defined in that DT (ad9361_clkin) is attached to a clock > > > that > > > has > > > a fixed rate (xo_40mhz_fixed_clk), that can become a variable rate > > > [from an > > > external source, when it is provided]. > > > > > > Our understanding, is that a GPIO gate clock should propagate the > > > rate > > > change to the parent clock. The same goes for the GPIO MUX clock. > > > So, the default mode would be to always set CLK_SET_RATE_PARENT. > > > > > > But, given that there are users of this driver, such a behavior > > > change > > > could break other users, so we are using this DT prop. > > > > > > There seems to be only one user of gpio-mux-clock in > > > arch/arm64/boot/dts/renesas/ulcb-kf.dtsi > > > > > > Whereas for gpio-gate-clock, there are multiple users. I can't say > > > whether > > > this change would break anything for them, or it would be fine. > > > > I think we can just add CLK_SET_RATE_PARENT for all gpio gate clks then > > and you'll be happy? Care to send that patch and Cc Geert and other > > Renesas maintainers? That avoids any DT property and should be fine for > > anyone as far as I can tell. Maybe the inverse will be desired, but not > > wanting rate to propagate is an edge case that we may want to call out > > explicitly in DT at some point in the future, but shouldn't worry about > > until then. > > > > Will resend, thanks. > Geert is CC-ed here as well [unless there is another Geert].
Oh right. You CC-ed Geert this time. I assumed that I did, from the get_maintainers script. It's still seems like black-art trying to see how tag people when sending patches. Especially if you're new.
> > I'll send a V2 in-reply-to this thread. > > Thanks > Alex
| |