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

[skannan@codeaurora.org]

On 2018-08-02 05:07, Georgi Djakov wrote:
> Hi Saravana,
> 
> On 08/02/2018 01:57 AM, skannan@codeaurora.org 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@linaro.org>
>>> ---
>>>  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