Re: [PATCH v3 2/4] clk: core: link consumer with clock driver

[Miquel Raynal]

Hi Stephen,

Stephen Boyd <sboyd@xxxxxxxxxx> wrote on Fri, 25 Jan 2019 13:28:28
-0800:

> Quoting Miquel Raynal (2019-01-04 07:54:06)
> > Hi Stephen,
> > 
> > Stephen Boyd <sboyd@xxxxxxxxxx> wrote on Tue, 11 Dec 2018 09:12:55
> > -0800:
> >   
> > > Sorry, I'm not reviewing the whole patch right now, just this one little
> > > bit because I'm also working in the same area.
> > > 
> > > Quoting Miquel Raynal (2018-12-04 11:24:38)  
> > > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > > > index 60c51871b04b..721d6b55b2fa 100644
> > > > --- a/include/linux/clk-provider.h
> > > > +++ b/include/linux/clk-provider.h
> > > > @@ -781,6 +781,8 @@ void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw);
> > > >  const char *__clk_get_name(const struct clk *clk);
> > > >  const char *clk_hw_get_name(const struct clk_hw *hw);
> > > >  struct clk_hw *__clk_get_hw(struct clk *clk);
> > > > +void clk_link_consumer(struct device *consumer, struct clk *clk);
> > > > +void clk_unlink_consumer(struct clk *clk);    
> > > 
> > > We shouldn't need to add these functions as far as I can tell. That's
> > > because __clk_get() has become an internal API between clkdev.c and
> > > clk.c that does nothing now on implementations that aren't the CCF. We
> > > can even change this API to take a clk_hw pointer instead of a clk
> > > pointer.
> > > 
> > > I'd rather see us plumb a struct device and clk_hw structure down into
> > > __clk_get() and fold it all into __clk_create_clk, possibly even
> > > renaming __clk_create_clk to clk_hw_create_clk(). That way we can get
> > > the calling device and clk_hw pointer in one call in the clk framework,
> > > along with the device name and connection name, and then generate the
> > > struct clk right there. This can simplify some code and make it easier
> > > to extend this to associate calling devices with the clk consumer
> > > somehow.
> > > 
> > > Here's the diff. With this, you should be able to add and remove device
> > > links in clk_hw_create_clk() when dev != NULL.  
> > 
> > Thanks for the help; I updated my work on top of yours, it looks ok but
> > I need to run some more tests.
> > 
> > However I had to tweak a parameter in one of your recent changes, you
> > used '-1' as index in __of_clock_get() while it is not a valid value
> > (returning an error). As in the __of_clk_get_by_name() function you
> > removed index was just set to 0 at the top of the function, I think the
> > below fix is valid.  
> 
> Thanks. Makes sense so I folded it in.

Great!

Can you update me with the status of this series? Is it going in for 5.1?

Lorenzo, the PCI maintainer, is waiting for this series to enter before
accepting another series depending on it; so please do not hesitate to
keep me updated.


Thanks,
MiquÃl