[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <599471308374b786af4dc8a6b42fea76@manjaro.org>
Date: Fri, 08 Nov 2024 15:18:57 +0100
From: Dragan Simic <dsimic@...jaro.org>
To: Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
heiko@...ech.de, hjc@...k-chips.com, andy.yan@...k-chips.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, simona@...ll.ch, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Diederik de Haas <didi.debian@...ow.org>
Subject: Re: [PATCH 2/2] drm/rockchip: dsi: Don't log errors on deferred dphy
Hello Sebastian,
On 2024-11-08 15:08, Sebastian Reichel wrote:
> On Fri, Nov 08, 2024 at 02:53:58PM +0100, Dragan Simic wrote:
>> Deferred driver probing shouldn't result in errors or warnings being
>> logged,
>> because their presence in the kernel log provides no value and may
>> actually
>> cause false impression that some issues exist. Thus, let's no longer
>> produce
>> error messages when getting the dphy results in deferred probing.
>>
>> This prevents misleading error messages like the following one, which
>> was
>> observed on a Pine64 PineTab2, from appearing in the kernel log. To
>> make
>> matters worse, the following error message was observed appearing
>> multiple
>> times in the kernel log of a single PineTab2 boot:
>>
>> dw-mipi-dsi-rockchip fe060000.dsi: [drm:dw_mipi_dsi_rockchip_probe \
>> [rockchipdrm]] *ERROR* failed to get mipi dphy: -517
>>
>> At the same time, make the adjusted logged message a bit more
>> consistent with
>> the other logged messages by capitalizing its first word.
>>
>> Reported-by: Diederik de Haas <didi.debian@...ow.org>
>> Signed-off-by: Dragan Simic <dsimic@...jaro.org>
>> ---
>
> From include/drm/drm_print.h:
>
> * DRM_DEV_ERROR() - Error output.
> *
> * NOTE: this is deprecated in favor of drm_err() or dev_err().
>
> The recommended way to do this nowadays looks like this:
>
> return dev_err_probe(dev, PTR_ERR(dsi->phy), "Failed to get mipi
> dphy");
>
> That will not print anything for -EPROBE_DEFER, but capture
> the reason and make it available through
> /sys/kernel/debug/devices_deferred if the device never probes.
Thanks for your quick response! As already discussed with Heiko,
I'll move forward with implementing a complete file-level conversion.
At first, I thought that a partial bugfix would be beneficial, [1]
but now I agree that performing a complete file-level coversion is
the way to go. [2]
I've got to admit, I love seeing that DRM_DEV_ERROR() is deprecated,
because I've never been a big fan of the format of the messages
it produces.
[1]
https://lore.kernel.org/dri-devel/3734f6a5424e3537d717c587a058fc85@manjaro.org/
[2]
https://lore.kernel.org/dri-devel/047164cc6e88dcbc7701cb0e28d564db@manjaro.org/
>> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> index f451e70efbdd..ffa7f2bc640d 100644
>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> @@ -1387,7 +1387,8 @@ static int dw_mipi_dsi_rockchip_probe(struct
>> platform_device *pdev)
>> dsi->phy = devm_phy_optional_get(dev, "dphy");
>> if (IS_ERR(dsi->phy)) {
>> ret = PTR_ERR(dsi->phy);
>> - DRM_DEV_ERROR(dev, "failed to get mipi dphy: %d\n", ret);
>> + if (ret != -EPROBE_DEFER)
>> + DRM_DEV_ERROR(dev, "Failed to get mipi dphy: %d\n", ret);
>> return ret;
>> }
>>
Powered by blists - more mailing lists