Re: [PATCH v7 8/8] interconnect: Allow endpoints translation via DT

[skannan]

On 2018-08-02 05:07, Georgi Djakov wrote:
> Hi Saravana,
>
> On 08/02/2018 01:57 AM, skannan@xxxxxxxxxxxxxx wrote:
> > On 2018-07-31 09:13, Georgi Djakov wrote:
> > > Currently we support only platform data for specifying the 
> > > interconnect
> > > endpoints. As now the endpoints are hard-coded into the consumer 
> > > driver
> > > this may lead to complications when a single driver is used by 
> > > multiple
> > > SoCs, which may have different interconnect topology.
> > > To avoid cluttering the consumer drivers, introduce a translation
> > > function
> > > to help us get the board specific interconnect data from device-tree.
> > >
> > > Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
> > > ---
> > > Âdrivers/interconnect/core.c | 62 
> > > ++++++++++++++++++++++++++++++++++++
> > > Âinclude/linux/interconnect.h |Â 7 ++++
> > > Â2 files changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/interconnect/core.c 
> > > b/drivers/interconnect/core.c
> > > index 9fef180cf77e..d1b6adff0a3d 100644
> > > --- a/drivers/interconnect/core.c
> > > +++ b/drivers/interconnect/core.c
> [..]
> > > --- a/include/linux/interconnect.h
> > > +++ b/include/linux/interconnect.h
> > > @@ -17,6 +17,7 @@ struct device;
> > >
> > > Âstruct icc_path *icc_get(struct device *dev, const int src_id,
> > > ÂÂÂÂÂÂÂÂÂÂÂÂÂ const int dst_id);
> > > +struct icc_path *of_icc_get(struct device *dev, const char *name);
> > > Âvoid icc_put(struct icc_path *path);
> > > Âint icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> > >
> > > @@ -28,6 +29,12 @@ static inline struct icc_path *icc_get(struct
> > > device *dev, const int src_id,
> > > ÂÂÂÂ return NULL;
> > > Â}
> > >
> > > +static inline struct icc_path *of_icc_get(struct device *dev,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *name)
> > > +{
> > > +ÂÂÂ return NULL;
> > > +}
> > > +
> >
> > Might want to return PTR(-ENODEV) or some error code so that client
> > doesn't have to do NULL check AND an error check?
> >
> > -Saravana
>
> NULL is returned when CONFIG_INTERCONNECT=n. Configuration of
> interconnects by consumer drivers could be optional and that's why null
> is returned instead of an error. The consumer drivers decide how to
> proceed in this case and if there is a hard requirement for 
> interconnect
> support, then i would suggest to express it as a dependency in Kconfig.

Ehh... you could make the same argument with an error. If it's not 
mandatory for functioning, they can ignore a specific error and 
continue? At a minimum, these stub functions returning NULL doesn't 
match with the documentation that says these APIs will only ever return 
ERR_PTR().

-Saravana