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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <37ac189618cda02567977f65570d15d3@codeaurora.org>
Date:   Thu, 02 Aug 2018 12:12:15 -0700
From:   skannan@...eaurora.org
To:     Georgi Djakov <georgi.djakov@...aro.org>
Cc:     linux-pm@...r.kernel.org, gregkh@...uxfoundation.org,
        rjw@...ysocki.net, robh+dt@...nel.org, mturquette@...libre.com,
        khilman@...libre.com, vincent.guittot@...aro.org,
        bjorn.andersson@...aro.org, amit.kucheria@...aro.org,
        seansw@....qualcomm.com, daidavid1@...eaurora.org,
        evgreen@...omium.org, mark.rutland@....com,
        lorenzo.pieralisi@....com, abailon@...libre.com, arnd@...db.de,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v7 8/8] interconnect: Allow endpoints translation via DT

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