All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>,
	jirislaby@kernel.org, LinoSanfilippo@gmx.de, lukas@wunner.de,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Date: Thu, 18 Jan 2024 09:44:19 +0100	[thread overview]
Message-ID: <2024011824-squiggle-foil-8db4@gregkh> (raw)
In-Reply-To: <Zajd8fd/U3foRyLB@moxa-ThinkCentre-M90t>

On Thu, Jan 18, 2024 at 04:14:41PM +0800, Crescent CY Hsieh wrote:
> On Thu, Jan 18, 2024 at 08:01:58AM +0100, Greg KH wrote:
> > On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote:
> > > >  struct serial_rs485 {
> > > >  	__u32	flags;
> > > > -#define SER_RS485_ENABLED		(1 << 0)
> > > > -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> > > > -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> > > 
> > > In the old definition (1 << 3) wasn't used.
> > > 
> > > > -#define SER_RS485_RX_DURING_TX		(1 << 4)
> > > > -#define SER_RS485_TERMINATE_BUS		(1 << 5)
> > > > -#define SER_RS485_ADDRB			(1 << 6)
> > > > -#define SER_RS485_ADDR_RECV		(1 << 7)
> > > > -#define SER_RS485_ADDR_DEST		(1 << 8)
> > > > +#define SER_RS485_ENABLED		_BITUL(0)
> > > > +#define SER_RS485_RTS_ON_SEND		_BITUL(1)
> > > > +#define SER_RS485_RTS_AFTER_SEND	_BITUL(2)
> > > > +#define SER_RS485_RX_DURING_TX		_BITUL(3)
> > > 
> > > Isn't it a break if number 3 isn't skipped here as well?
> 
> Sorry I might have misunderstood the meaning of "broke userspace".
> 
> In this case, does it imply splitting the "cleanup" patch and the "add
> feature" patch, or leaving the third bit unused? Or perhaps both?

You can not redefine existing defines to different values, like you did
here, that changes the user/kernel api of the system.

Luckily this isn't in any released kernel, I'll either revert this after
-rc1 is out, or take a patch from anyone else to do so if they want to
send it now.

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@linuxfoundation.org>
To: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>,
	jirislaby@kernel.org, LinoSanfilippo@gmx.de, lukas@wunner.de,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro
Date: Thu, 18 Jan 2024 09:44:19 +0100	[thread overview]
Message-ID: <2024011824-squiggle-foil-8db4@gregkh> (raw)
In-Reply-To: <Zajd8fd/U3foRyLB@moxa-ThinkCentre-M90t>

On Thu, Jan 18, 2024 at 04:14:41PM +0800, Crescent CY Hsieh wrote:
> On Thu, Jan 18, 2024 at 08:01:58AM +0100, Greg KH wrote:
> > On Wed, Jan 17, 2024 at 03:56:23PM +0100, Christoph Niedermaier wrote:
> > > >  struct serial_rs485 {
> > > >  	__u32	flags;
> > > > -#define SER_RS485_ENABLED		(1 << 0)
> > > > -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> > > > -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> > > 
> > > In the old definition (1 << 3) wasn't used.
> > > 
> > > > -#define SER_RS485_RX_DURING_TX		(1 << 4)
> > > > -#define SER_RS485_TERMINATE_BUS		(1 << 5)
> > > > -#define SER_RS485_ADDRB			(1 << 6)
> > > > -#define SER_RS485_ADDR_RECV		(1 << 7)
> > > > -#define SER_RS485_ADDR_DEST		(1 << 8)
> > > > +#define SER_RS485_ENABLED		_BITUL(0)
> > > > +#define SER_RS485_RTS_ON_SEND		_BITUL(1)
> > > > +#define SER_RS485_RTS_AFTER_SEND	_BITUL(2)
> > > > +#define SER_RS485_RX_DURING_TX		_BITUL(3)
> > > 
> > > Isn't it a break if number 3 isn't skipped here as well?
> 
> Sorry I might have misunderstood the meaning of "broke userspace".
> 
> In this case, does it imply splitting the "cleanup" patch and the "add
> feature" patch, or leaving the third bit unused? Or perhaps both?

You can not redefine existing defines to different values, like you did
here, that changes the user/kernel api of the system.

Luckily this isn't in any released kernel, I'll either revert this after
-rc1 is out, or take a patch from anyone else to do so if they want to
send it now.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-01-18  8:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-01  7:15 [PATCH v6 0/2] tty: serial: Add RS422 serial interface Crescent CY Hsieh
2023-12-01  7:15 ` [PATCH v6 1/2] tty: serial: Cleanup the bit shift with macro Crescent CY Hsieh
2024-01-16 15:58   ` Christoph Niedermaier
2024-01-16 15:58   ` Christoph Niedermaier
2024-01-17 14:56   ` Christoph Niedermaier
2024-01-17 14:56     ` Christoph Niedermaier
2024-01-18  7:01     ` Greg KH
2024-01-18  7:01       ` Greg KH
2024-01-18  8:14       ` Crescent CY Hsieh
2024-01-18  8:14         ` Crescent CY Hsieh
2024-01-18  8:44         ` Greg KH [this message]
2024-01-18  8:44           ` Greg KH
2024-01-18  9:29       ` Christoph Niedermaier
2024-01-18  9:29         ` Christoph Niedermaier
2023-12-01  7:15 ` [PATCH v6 2/2] tty: serial: Add RS422 flag to struct serial_rs485 Crescent CY Hsieh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2024011824-squiggle-foil-8db4@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=LinoSanfilippo@gmx.de \
    --cc=cniedermaier@dh-electronics.com \
    --cc=crescentcy.hsieh@moxa.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=lukas@wunner.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.