[PATCH] auxdisplay: panel: replace struct with union for display configuration

[PATCH] auxdisplay: panel: replace struct with union for display configuration

[Atharva Tiwari]

this patch replaces a struct with a union in the panel.c driver 
to better represent display configuration as mentioned in TODO

Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
---
 drivers/auxdisplay/panel.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index a731f28455b4..4662f763dac7 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -204,8 +204,7 @@ static struct {
 	int charset;
 	int proto;
 
-	/* TODO: use union here? */
-	struct {
+	union {
 		int e;
 		int rs;
 		int rw;
-- 
2.39.5

Re: [PATCH] auxdisplay: panel: replace struct with union for display configuration

[Willy TARREAU]

Hello!

On Wed, Dec 25, 2024 at 11:11:18PM +0530, Atharva Tiwari wrote:
> this patch replaces a struct with a union in the panel.c driver 
> to better represent display configuration as mentioned in TODO
> 
> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
> ---
>  drivers/auxdisplay/panel.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> index a731f28455b4..4662f763dac7 100644
> --- a/drivers/auxdisplay/panel.c
> +++ b/drivers/auxdisplay/panel.c
> @@ -204,8 +204,7 @@ static struct {
>  	int charset;
>  	int proto;
>  
> -	/* TODO: use union here? */
> -	struct {
> +	union {
>  		int e;
>  		int rs;
>  		int rw;

Have you tested this patch ? I guess not. The TODO here is not just to
change a language keyword but to see if it would be better achieved
using a different construct and representation of the different signals.
Here what you've done is merge all the signals into a single one.

I think that a better patch would be to just remove the TODO comment
that has been there for about 20 years without making any progress, and
which is no longer relevant since that code will not change now.

regards,
Willy

[PATCH] auxdisplay: panel: replace struct with union for display configuration

[Atharva Tiwari]

Well as you said it was better to just remove the todo that just what i
did :)

Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
---
 drivers/auxdisplay/panel.c | 1 +--
 1 file changed, 1 deletions(-)

diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index a731f28455b4..4662f763dac7 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -204,8 +204,7 @@ static struct {
 	int charset;
 	int proto;
 
-	/* TODO: use union here? */
 		int e;
 		int rs;
 		int rw;
-- 
2.39.5

Re: [PATCH] auxdisplay: panel: replace struct with union for display configuration

[Andy Shevchenko]

On Wed, Dec 25, 2024 at 8:06 PM Atharva Tiwari <evepolonium@gmail.com> wrote:
>
> Well as you said it was better to just remove the todo that just what i
> did :)

Please, follow the "Submitting Patches" and "Linux Kernel patch
submission checklist" documentation and try again after studying that.

-- 
With Best Regards,
Andy Shevchenko