[PATCH] drm/exynos: gsc: Get device id from OF alias

[Andrzej Hajda]

On 20.06.2018 13:53, Krzysztof Kozlowski wrote:
> On 20 June 2018 at 13:38, Andrzej Hajda <a.hajda at samsung.com> wrote:
>> Hi Krzysztof,
>>
>> On 20.06.2018 12:40, Krzysztof Kozlowski wrote:
>>> On 15 June 2018 at 14:26, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>>>> Platform devices instantiated from device-tree always have pdev->id set to
>>>> -1, so use of_get_alias_id() helper to retrieve proper device id.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_drm_gsc.c | 9 +++++++--
>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> index e99dd1e4ba65..a63287597985 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
>>>> @@ -1196,7 +1196,12 @@ static int gsc_probe(struct platform_device *pdev)
>>>>         struct exynos_drm_ipp_formats *formats;
>>>>         struct gsc_context *ctx;
>>>>         struct resource *res;
>>>> -       int ret, i;
>>>> +       int ret, i, id;
>>>> +
>>>> +       ret = of_alias_get_id(pdev->dev.of_node, "gsc");
>>>> +       if (ret < 0)
>>>> +               return ret;
>>>> +       id = ret;
>>>>
>>>>         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>>         if (!ctx)
>>>> @@ -1254,7 +1259,7 @@ static int gsc_probe(struct platform_device *pdev)
>>>>         }
>>>>
>>>>         /* context initailization */
>>>> -       ctx->id = pdev->id;
>>>> +       ctx->id = id;
>>> Why do you need ctx->id at all? I see it is used only in dev_dbg and
>>> dev_err messages but these should be easily identifiable by device
>>> name+address. Maybe get rid of ctx->id entirely?
>> I am working on patches adding framebuffer display pre-processing
>> on-the-fly, they requires gscaler id to program sysreg registers. I hope
>> to post it in near future.
> OK, makes sense. Then I have a dependent comment - if alias id is used
> by driver then probably it should be validated to prevent errors like
> out-of-bounds access. DTB theoretically might come from out-of-tree or
> from older version. Something like this was reported for our pinctrl
> driver:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93b0beae721b3344923b4b8317e9d83b542f4ca6
> Probably the validation should come with your code, Andrzej. Not here.

OK, I will add it in my patchset.

Regards
Andrzej

>
> Best regards,
> Krzysztof
>
>
>