* [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages
@ 2025-05-09 7:58 Krzysztof Hałasa
2025-05-09 9:15 ` Laurent Pinchart
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Hałasa @ 2025-05-09 7:58 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, imx,
linux-arm-kernel, linux-kernel
I guess these messages were split from a single function. Or was it
for saving memory? :-)
Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 93a55c97cd17..b6c7f114f9ed 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -352,9 +352,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
sink_streams);
if (ret) {
dev_err(xbar->isi->dev,
- "failed to %s streams 0x%llx on '%s':%u: %d\n",
- "enable", sink_streams, remote_sd->name,
- remote_pad, ret);
+ "failed to enable streams 0x%llx on '%s':%u: %d\n",
+ sink_streams, remote_sd->name, remote_pad, ret);
mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
return ret;
}
@@ -392,9 +391,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
sink_streams);
if (ret)
dev_err(xbar->isi->dev,
- "failed to %s streams 0x%llx on '%s':%u: %d\n",
- "disable", sink_streams, remote_sd->name,
- remote_pad, ret);
+ "failed to disable streams 0x%llx on '%s':%u: %d\n",
+ sink_streams, remote_sd->name, remote_pad, ret);
mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
}
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages
2025-05-09 7:58 [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages Krzysztof Hałasa
@ 2025-05-09 9:15 ` Laurent Pinchart
2025-05-20 11:19 ` Krzysztof Hałasa
0 siblings, 1 reply; 4+ messages in thread
From: Laurent Pinchart @ 2025-05-09 9:15 UTC (permalink / raw)
To: Krzysztof Hałasa
Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, imx,
linux-arm-kernel, linux-kernel
Hi Krzysztof,
On Fri, May 09, 2025 at 09:58:09AM +0200, Krzysztof Hałasa wrote:
> I guess these messages were split from a single function. Or was it
> for saving memory? :-)
The goal was indeed to save memory.
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> index 93a55c97cd17..b6c7f114f9ed 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -352,9 +352,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
> sink_streams);
> if (ret) {
> dev_err(xbar->isi->dev,
> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
> - "enable", sink_streams, remote_sd->name,
> - remote_pad, ret);
> + "failed to enable streams 0x%llx on '%s':%u: %d\n",
> + sink_streams, remote_sd->name, remote_pad, ret);
> mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> return ret;
> }
> @@ -392,9 +391,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
> sink_streams);
> if (ret)
> dev_err(xbar->isi->dev,
> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
> - "disable", sink_streams, remote_sd->name,
> - remote_pad, ret);
> + "failed to disable streams 0x%llx on '%s':%u: %d\n",
> + sink_streams, remote_sd->name, remote_pad, ret);
>
> mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages
2025-05-09 9:15 ` Laurent Pinchart
@ 2025-05-20 11:19 ` Krzysztof Hałasa
2025-05-21 10:29 ` Laurent Pinchart
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Hałasa @ 2025-05-20 11:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, imx,
linux-arm-kernel, linux-kernel
Hi Laurent,
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> The goal was indeed to save memory.
>
>> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
>> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
>> @@ -352,9 +352,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
>> sink_streams);
>> if (ret) {
>> dev_err(xbar->isi->dev,
>> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
>> - "enable", sink_streams, remote_sd->name,
>> - remote_pad, ret);
>> + "failed to enable streams 0x%llx on '%s':%u: %d\n",
>> + sink_streams, remote_sd->name, remote_pad, ret);
>> mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
>> return ret;
>> }
>> @@ -392,9 +391,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
>> sink_streams);
>> if (ret)
>> dev_err(xbar->isi->dev,
>> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
>> - "disable", sink_streams, remote_sd->name,
>> - remote_pad, ret);
>> + "failed to disable streams 0x%llx on '%s':%u: %d\n",
>> + sink_streams, remote_sd->name, remote_pad, ret);
It appears the current code saves (in my default build) 8 bytes of
memory, at the cost of readability and inability to search with grep:
Current:
Name Size File off
.text 00000bf4 00000040
.data 00000000 00000c34
.rodata.str1.8 000001b8 00000c38
__jump_table 00000030 00000df0
.rodata 000001b8 00000e20
With patch:
Name Size File off
.text 00000bd4 00000040
.data 00000000 00000c14
.rodata.str1.8 000001e0 00000c18
__jump_table 00000030 00000df8
.rodata 000001b8 00000e28
Built as a module it doesn't even save that - the number of pages stays
the same.
Just FYI.
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages
2025-05-20 11:19 ` Krzysztof Hałasa
@ 2025-05-21 10:29 ` Laurent Pinchart
0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2025-05-21 10:29 UTC (permalink / raw)
To: Krzysztof Hałasa
Cc: Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-media, imx,
linux-arm-kernel, linux-kernel
Hi Krzysztof,
On Tue, May 20, 2025 at 01:19:12PM +0200, Krzysztof Hałasa wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>
> > The goal was indeed to save memory.
> >
> >> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> >> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> >> @@ -352,9 +352,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
> >> sink_streams);
> >> if (ret) {
> >> dev_err(xbar->isi->dev,
> >> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
> >> - "enable", sink_streams, remote_sd->name,
> >> - remote_pad, ret);
> >> + "failed to enable streams 0x%llx on '%s':%u: %d\n",
> >> + sink_streams, remote_sd->name, remote_pad, ret);
> >> mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> >> return ret;
> >> }
> >> @@ -392,9 +391,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
> >> sink_streams);
> >> if (ret)
> >> dev_err(xbar->isi->dev,
> >> - "failed to %s streams 0x%llx on '%s':%u: %d\n",
> >> - "disable", sink_streams, remote_sd->name,
> >> - remote_pad, ret);
> >> + "failed to disable streams 0x%llx on '%s':%u: %d\n",
> >> + sink_streams, remote_sd->name, remote_pad, ret);
>
> It appears the current code saves (in my default build) 8 bytes of
> memory, at the cost of readability and inability to search with grep:
>
> Current:
> Name Size File off
> .text 00000bf4 00000040
> .data 00000000 00000c34
> .rodata.str1.8 000001b8 00000c38
> __jump_table 00000030 00000df0
> .rodata 000001b8 00000e20
>
> With patch:
> Name Size File off
> .text 00000bd4 00000040
> .data 00000000 00000c14
> .rodata.str1.8 000001e0 00000c18
> __jump_table 00000030 00000df8
> .rodata 000001b8 00000e28
>
> Built as a module it doesn't even save that - the number of pages stays
> the same.
>
> Just FYI.
Fair enough. I'll take the patch, with an update to the commit message:
media: nxp: imx8-isi: Simplify a couple of error messages
The error messages in the mxc_isi_crossbar_enable_streams() and
mxc_isi_crossbar_disable_streams() functions are similar, with a single
word difference between them. The word is moved out of the format string
to a separate dev_err() argument to try and save memory through string
de-duplication. The total savings are however small, as the .data size
reduction is partly offset by a .text size increase, with a total saving
of 8 bytes in total on an ARM64 platforms. They also come at the cost of
making the error message difficult to grep, which outweights the gains.
Simplify the error messages to make them grep-able.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-05-21 10:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-09 7:58 [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages Krzysztof Hałasa
2025-05-09 9:15 ` Laurent Pinchart
2025-05-20 11:19 ` Krzysztof Hałasa
2025-05-21 10:29 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).