Re: [PATCH v4 1/3] tty: serial: samsung: Use BIT() macro for APPLE_S5L_*

[Andi Shyti]

Hi Nick,

On Tue, Sep 10, 2024 at 10:26:44PM GMT, Nick Chan wrote:
> On 10/9/2024 20:48, Andi Shyti wrote:
> > On Mon, Sep 09, 2024 at 04:37:25PM GMT, Nick Chan wrote:
> >> New entries using BIT() will be added soon, so change the existing ones
> >> for consistency.
> >>
> >> Signed-off-by: Nick Chan <towinchenmi@xxxxxxxxx>
> > 
> > I think this is:
> > 
> > Suggested-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> We will see... Got a bit paranoid after bad things happened with v2 and v3.

ahaha... in this case it's fine. The whole patch was suggested by
Krzysztof, so that it makes sense to add this tag.

What Krzysztof complained about is that you accidentally added
his r-b without him telling you explicitely.

> > 
> >> ---
> >>  include/linux/serial_s3c.h | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h
> >> index 1672cf0810ef..1e8686695487 100644
> >> --- a/include/linux/serial_s3c.h
> >> +++ b/include/linux/serial_s3c.h
> >> @@ -249,9 +249,9 @@
> >>  #define APPLE_S5L_UCON_RXTO_ENA		9
> >>  #define APPLE_S5L_UCON_RXTHRESH_ENA	12
> >>  #define APPLE_S5L_UCON_TXTHRESH_ENA	13
> >> -#define APPLE_S5L_UCON_RXTO_ENA_MSK	(1 << APPLE_S5L_UCON_RXTO_ENA)
> >> -#define APPLE_S5L_UCON_RXTHRESH_ENA_MSK	(1 << APPLE_S5L_UCON_RXTHRESH_ENA)
> >> -#define APPLE_S5L_UCON_TXTHRESH_ENA_MSK	(1 << APPLE_S5L_UCON_TXTHRESH_ENA)
> >> +#define APPLE_S5L_UCON_RXTO_ENA_MSK	BIT(APPLE_S5L_UCON_RXTO_ENA)
> >> +#define APPLE_S5L_UCON_RXTHRESH_ENA_MSK	BIT(APPLE_S5L_UCON_RXTHRESH_ENA)
> >> +#define APPLE_S5L_UCON_TXTHRESH_ENA_MSK	BIT(APPLE_S5L_UCON_TXTHRESH_ENA)
> >>  
> >>  #define APPLE_S5L_UCON_DEFAULT		(S3C2410_UCON_TXIRQMODE | \
> >>  					 S3C2410_UCON_RXIRQMODE | \
> >> @@ -260,9 +260,9 @@
> >>  					 APPLE_S5L_UCON_RXTHRESH_ENA_MSK | \
> >>  					 APPLE_S5L_UCON_TXTHRESH_ENA_MSK)
> >>  
> >> -#define APPLE_S5L_UTRSTAT_RXTHRESH	(1<<4)
> >> -#define APPLE_S5L_UTRSTAT_TXTHRESH	(1<<5)
> >> -#define APPLE_S5L_UTRSTAT_RXTO		(1<<9)
> >> +#define APPLE_S5L_UTRSTAT_RXTHRESH	BIT(4)
> >> +#define APPLE_S5L_UTRSTAT_TXTHRESH	BIT(5)
> >> +#define APPLE_S5L_UTRSTAT_RXTO		BIT(9)
> >>  #define APPLE_S5L_UTRSTAT_ALL_FLAGS	(0x3f0)
> > 
> > You could make this GENMASK(0x3f, 4)
> Good idea, given the above context I think I may add
> 
> Suggested-by: Andi Shyti <andi.shyti@xxxxxxxxxx>

ehm... not in this case. Mine is a suggestion as reviewer and
this little comment does not deserve a "Suggested-by" like
in Krzysztof's case.

> too. And actually it should be GENMASK(9, 3)

You find out the right parameters :-)

Thanks,
Andi

> > 
> > Andi
> 
> Nick Chan
>