Re: [PATCH v7 5/6] serial: Support for RS-485 multipoint addresses

[Jiri Slaby]

On 16. 06. 22, 7:04, Ilpo Järvinen wrote:
> On Wed, 15 Jun 2022, Andy Shevchenko wrote:
>
> > On Wed, Jun 15, 2022 at 03:48:28PM +0300, Ilpo Järvinen wrote:
> > > Add support for RS-485 multipoint addressing using 9th bit [*]. The
> > > addressing mode is configured through .rs485_config().
> > >
> > > ADDRB in termios indicates 9th bit addressing mode is enabled. In this
> > > mode, 9th bit is used to indicate an address (byte) within the
> > > communication line. ADDRB can only be enabled/disabled through
> > > .rs485_config() that is also responsible for setting the destination and
> > > receiver (filter) addresses.
> > >
> > > [*] Technically, RS485 is just an electronic spec and does not itself
> > > specify the 9th bit addressing mode but 9th bit seems at least
> > > "semi-standard" way to do addressing with RS485.
> > >
> > > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > > Cc: Jonathan Corbet <corbet@xxxxxxx>
> > > Cc: linux-api@xxxxxxxxxxxxxxx
> > > Cc: linux-doc@xxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > Cc: linux-arch@xxxxxxxxxxxxxxx
> >
> > Hmm... In order to reduce commit messages you can move these Cc:s after the
> > cutter line ('---').
>
> Ok, although the toolchain I use didn't support preserving --- content
> so I had to create hack to preserve them, hopefully nothing backfires due
> to the hack. :-)
>
> > > -	__u32	padding[5];		/* Memory is cheap, new structs
> > > -					   are a royal PITA .. */
> > > +	__u8	addr_recv;
> > > +	__u8	addr_dest;
> > > +	__u8	padding[2 + 4 * sizeof(__u32)];		/* Memory is cheap, new structs
> > > +							 * are a royal PITA .. */
> >
> > I'm not sure it's an equivalent. I would leave u32 members  untouched, so
> > something like
> >
> > 	__u8	addr_recv;
> > 	__u8	addr_dest;
> > 	__u8	padding0[2];		/* Memory is cheap, new structs
> > 	__u32	padding1[4];		 * are a royal PITA .. */
> >
> > And repeating about `pahole` tool which may be useful here to check for ABI
> > potential changes.
>
> I cannot take __u32 padding[] away like that, this is an uapi header.

Yeah, but it's padding after all. I would personally break it for 
example as Andy suggests (if pahole shows no differences in size on both 
32/64 bit) and wait if something breaks. To be honest, I'd not expect 
anyone to touch it. And if someone does, we would fix it somehow and 
they should too...

thanks,
--
js
suse labs