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

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

[Marek Szyprowski]

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@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;
 
 	platform_set_drvdata(pdev, ctx);
 
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Krzysztof Kozlowski]

On 15 June 2018 at 14:26, Marek Szyprowski <m.szyprowski@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@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?

Best regards,
Krzysztof
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Andrzej Hajda]

Hi Krzysztof,

On 20.06.2018 12:40, Krzysztof Kozlowski wrote:
> On 15 June 2018 at 14:26, Marek Szyprowski <m.szyprowski@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@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.

Regards
Andrzej

>
> Best regards,
> Krzysztof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Krzysztof Kozlowski]

On 20 June 2018 at 13:38, Andrzej Hajda <a.hajda@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@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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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@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@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@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
>
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel