Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CGME20191220120143eucas1p1c9b01ae8c2e4ecd70423ef9d8001536f@eucas1p1.samsung.com>
 <20191220115653.6487-1-a.swigon@samsung.com> <20191220115653.6487-4-a.swigon@samsung.com>
In-Reply-To: <20191220115653.6487-4-a.swigon@samsung.com>
From:   Chanwoo Choi <chanwoo@kernel.org>
Date:   Mon, 23 Dec 2019 02:08:41 +0900
Message-ID: <CAGTfZH0zfvPYtTv6v+5nq99Gd2PVtg+O20dwf2nbV2j1U0nxCQ@mail.gmail.com>
Subject: Re: [RFC PATCH v3 3/7] interconnect: Allow inter-provider pairs to be configured
To:     =?UTF-8?B?QXJ0dXIgxZp3aWdvxYQ=?= <a.swigon@samsung.com>
Cc:     devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Linux PM list <linux-pm@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        MyungJoo Ham <myungjoo.ham@samsung.com>, inki.dae@samsung.com,
        Seung-Woo Kim <sw0312.kim@samsung.com>,
        Georgi Djakov <georgi.djakov@linaro.org>,
        Leonard Crestez <leonard.crestez@nxp.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Krzysztof Kozlowski <krzk@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Fri, Dec 20, 2019 at 9:03 PM Artur =C5=9Awigo=C5=84 <a.swigon@samsung.co=
m> wrote:
>
> In the exynos-bus devfreq driver every bus is probed separately and is

IMHO, the patch description should specify the more general cause
why have to be changed. Actually, almost people might not know
the 'exynos-bus'. So, firstly, you have to specify the general cause
why this patch is necessary without 'exynos-bus' word and then
add the real use-case with 'exynos-bus' example.

> assigned a separate interconnect provider. However, the interconnect
> framework does not call the '->set' callback for pairs of nodes which
> belong to different providers.
>
> This patch adds support for a new boolean 'inter_set' field in struct
> icc_provider. Setting it to 'true' enables calling '->set' for
> inter-provider node pairs. All existing users of the interconnect
> framework allocate this structure with kzalloc, and are therefore
> unaffected.
>
> Signed-off-by: Artur =C5=9Awigo=C5=84 <a.swigon@samsung.com>
> ---
>  drivers/interconnect/core.c           | 11 +++++------
>  include/linux/interconnect-provider.h |  2 ++
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 74c68898a350..a28bd0f8a497 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -259,23 +259,22 @@ static int aggregate_requests(struct icc_node *node=
)
>  static int apply_constraints(struct icc_path *path)
>  {
>         struct icc_node *next, *prev =3D NULL;
> +       struct icc_provider *p;
>         int ret =3D -EINVAL;
>         int i;
>
>         for (i =3D 0; i < path->num_nodes; i++) {
>                 next =3D path->reqs[i].node;
> +               p =3D next->provider;
>
> -               /*
> -                * Both endpoints should be valid master-slave pairs of t=
he
> -                * same interconnect provider that will be configured.
> -                */
> -               if (!prev || next->provider !=3D prev->provider) {
> +               /* both endpoints should be valid master-slave pairs */
> +               if (!prev || (p !=3D prev->provider && !p->inter_set)) {
>                         prev =3D next;
>                         continue;
>                 }
>
>                 /* set the constraints */
> -               ret =3D next->provider->set(prev, next);
> +               ret =3D p->set(prev, next);
>                 if (ret)
>                         goto out;
>
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interc=
onnect-provider.h
> index cc965b8fab53..b6ae0ee686c5 100644
> --- a/include/linux/interconnect-provider.h
> +++ b/include/linux/interconnect-provider.h
> @@ -41,6 +41,7 @@ struct icc_node *of_icc_xlate_onecell(struct of_phandle=
_args *spec,
>   * @xlate: provider-specific callback for mapping nodes from phandle arg=
uments
>   * @dev: the device this interconnect provider belongs to
>   * @users: count of active users
> + * @inter_set: whether inter-provider pairs will be configured with @set
>   * @data: pointer to private data
>   */
>  struct icc_provider {
> @@ -53,6 +54,7 @@ struct icc_provider {
>         struct icc_node* (*xlate)(struct of_phandle_args *spec, void *dat=
a);
>         struct device           *dev;
>         int                     users;
> +       bool                    inter_set;
>         void                    *data;
>  };
>
> --
> 2.17.1
>


--=20
Best Regards,
Chanwoo Choi