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

[Krzysztof Kozlowski]

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.

Best regards,
Krzysztof