[PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[Lech Perczak]

When submitting previous, functional fixes, Tomasz Moń omitted those
two cosmetic patches, that kept lurking in our company tree - likely
by oversight. Let's submit them.

Signed-off-by: Lech Perczak <lech.perczak@camlingroup.com>
---
v4:
No changes to patch 1 and 2.
- Fix typo in commit message in patch 3.
- Revert interrupt source constants back to numerical representation.
- Add a line break to SC16IS7XX_LSR_BRK_ERROR_MASK definition.

v3:
No code changes in patches 1 and 2.
- Pick up Reviewed-by from Andy in patch 1
- Adjust commit message in patch 2
- Perform further cleanup in bit constants,
  use GENMASK for SC16IS7XX_IIR_* and reuse bit definitions in
  SC16IS7XX_LSR_BRK_ERROR_MASK in patch 3.

v2:
- Converted bitmask definitions to use BIT macro
  (thanks Jiri Slaby for the idea)
- Removed redundant comments in patch 2 altogether
- Fixed commit messages (thanks Andy Shevchenko for
  thorough review)

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Andy Shevchenko <andy@kernel.org>

Lech Perczak (3):
  serial: sc16is7xx: remove SC16IS7XX_MSR_DELTA_MASK
  serial: sc16is7xx: fix copy-paste errors in EFR_SWFLOWx_BIT constants
  serial: sc16is7xx: convert bitmask definitions to use BIT() macro

 drivers/tty/serial/sc16is7xx.c | 183 +++++++++++++++++----------------
 1 file changed, 94 insertions(+), 89 deletions(-)


base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
-- 
2.34.1

Re: [PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[Andy Shevchenko]

On Mon, Aug 26, 2024 at 05:40:28PM +0200, Lech Perczak wrote:
> When submitting previous, functional fixes, Tomasz Moń omitted those
> two cosmetic patches, that kept lurking in our company tree - likely
> by oversight. Let's submit them.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

I haven't looked into the details of the changes, but feels good.
What you can do to confirm is to run this via C preprocessor and
show the diff here or assure that it's empty.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[Lech Perczak]

Hi Andy,

W dniu 26.08.2024 o 19:40, Andy Shevchenko pisze:
> On Mon, Aug 26, 2024 at 05:40:28PM +0200, Lech Perczak wrote:
>> When submitting previous, functional fixes, Tomasz Moń omitted those
>> two cosmetic patches, that kept lurking in our company tree - likely
>> by oversight. Let's submit them.
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> 
> I haven't looked into the details of the changes, but feels good.
> What you can do to confirm is to run this via C preprocessor and
> show the diff here or assure that it's empty.
> 

Great tip for checking such changes the correctness.
Up to patch v2 there were no significant changes, as expected, but for patch 3,
diff is quite substantial, due to BIT() being more explicit,
than open-coded constants.

Only the single expansion of GENMASK proves very hard to analyze in the
diff - so I double-checked with a calculator,
though all BIT() expansions do match, as does the updated definition.
of SC16IS7XX_LSR_BRK_ERROR_MASK.

Anyway, for completeness here is the diff in full for completeness. It's huge 
and has lines way over 80 characters, but that's how it is.
I obtained inputs for the diff by 'make drivers/tty/serial/sc16is7xx.i'.

46,48c46,80
< # 1 "./include/linux/clk.h" 1
< # 12 "./include/linux/clk.h"
< # 1 "./include/linux/err.h" 1
---
> # 1 "./include/linux/bits.h" 1
> 
> 
> 
> 
> # 1 "./include/linux/const.h" 1
> 
> 
> 
> # 1 "./include/vdso/const.h" 1
> 
> 
> 
> 
> # 1 "./include/uapi/linux/const.h" 1
> # 6 "./include/vdso/const.h" 2
> # 5 "./include/linux/const.h" 2
> # 6 "./include/linux/bits.h" 2
> # 1 "./include/vdso/bits.h" 1
> # 7 "./include/linux/bits.h" 2
> # 1 "./include/uapi/linux/bits.h" 1
> # 8 "./include/linux/bits.h" 2
> # 1 "./arch/x86/include/uapi/asm/bitsperlong.h" 1
> # 11 "./arch/x86/include/uapi/asm/bitsperlong.h"
> # 1 "./include/asm-generic/bitsperlong.h" 1
> 
> 
> 
> 
> # 1 "./include/uapi/asm-generic/bitsperlong.h" 1
> # 6 "./include/asm-generic/bitsperlong.h" 2
> # 12 "./arch/x86/include/uapi/asm/bitsperlong.h" 2
> # 9 "./include/linux/bits.h" 2
> # 22 "./include/linux/bits.h"
> # 1 "./include/linux/build_bug.h" 1
99,117c131
< # 12 "./include/uapi/asm-generic/int-ll64.h"
< # 1 "./arch/x86/include/uapi/asm/bitsperlong.h" 1
< # 11 "./arch/x86/include/uapi/asm/bitsperlong.h"
< # 1 "./include/asm-generic/bitsperlong.h" 1
< 
< 
< 
< 
< # 1 "./include/uapi/asm-generic/bitsperlong.h" 1
< # 6 "./include/asm-generic/bitsperlong.h" 2
< # 12 "./arch/x86/include/uapi/asm/bitsperlong.h" 2
< # 13 "./include/uapi/asm-generic/int-ll64.h" 2
< 
< 
< 
< 
< 
< 
< 
---
> # 20 "./include/uapi/asm-generic/int-ll64.h"
534c548,558
< # 6 "./include/linux/err.h" 2
---
> # 6 "./include/linux/build_bug.h" 2
> # 23 "./include/linux/bits.h" 2
> # 14 "drivers/tty/serial/sc16is7xx.c" 2
> # 1 "./include/linux/clk.h" 1
> # 12 "./include/linux/clk.h"
> # 1 "./include/linux/err.h" 1
> 
> 
> 
> 
> 
608,624d631
< 
< 
< 
< 
< # 1 "./include/linux/const.h" 1
< 
< 
< 
< # 1 "./include/vdso/const.h" 1
< 
< 
< 
< 
< # 1 "./include/uapi/linux/const.h" 1
< # 6 "./include/vdso/const.h" 2
< # 5 "./include/linux/const.h" 2
< # 6 "./include/linux/align.h" 2
706,711d712
< 
< 
< 
< 
< # 1 "./include/linux/build_bug.h" 1
< # 6 "./include/linux/container_of.h" 2
720,721d720
< # 1 "./include/linux/bits.h" 1
< 
723,730d721
< 
< 
< 
< # 1 "./include/vdso/bits.h" 1
< # 7 "./include/linux/bits.h" 2
< # 1 "./include/uapi/linux/bits.h" 1
< # 8 "./include/linux/bits.h" 2
< # 7 "./include/linux/bitops.h" 2
21426c21417
< # 14 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 15 "drivers/tty/serial/sc16is7xx.c" 2
21491c21482
< # 15 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 16 "drivers/tty/serial/sc16is7xx.c" 2
48375c48366
< # 16 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 17 "drivers/tty/serial/sc16is7xx.c" 2
53866c53857
< # 18 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 19 "drivers/tty/serial/sc16is7xx.c" 2
54000c53991
< # 20 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 21 "drivers/tty/serial/sc16is7xx.c" 2
54680c54671
< # 24 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 25 "drivers/tty/serial/sc16is7xx.c" 2
62770c62761
< # 26 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 27 "drivers/tty/serial/sc16is7xx.c" 2
62833c62824
< # 30 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 31 "drivers/tty/serial/sc16is7xx.c" 2
62904c62895
< # 32 "drivers/tty/serial/sc16is7xx.c" 2
---
> # 33 "drivers/tty/serial/sc16is7xx.c" 2
62934,62935c62925,62926
< # 34 "drivers/tty/serial/sc16is7xx.c" 2
< # 315 "drivers/tty/serial/sc16is7xx.c"
---
> # 35 "drivers/tty/serial/sc16is7xx.c" 2
> # 320 "drivers/tty/serial/sc16is7xx.c"
63029,63030c63020,63021
<          (1 << 4),
<          on ? 0 : (1 << 4));
---
>          ((((1UL))) << (4)),
>          on ? 0 : ((((1UL))) << (4)));
63032c63023
< # 426 "drivers/tty/serial/sc16is7xx.c"
---
> # 431 "drivers/tty/serial/sc16is7xx.c"
63069c63060
<  one->config.flags |= (1 << 1);
---
>  one->config.flags |= ((((1UL))) << (1));
63082c63073
<  one->config.flags |= (1 << 1);
---
>  one->config.flags |= ((((1UL))) << (1));
63090c63081
<  sc16is7xx_ier_clear(port, (1 << 1));
---
>  sc16is7xx_ier_clear(port, ((((1UL))) << (1)));
63095c63086
<  sc16is7xx_ier_clear(port, (1 << 0));
---
>  sc16is7xx_ier_clear(port, ((((1UL))) << (0)));
63164c63155
< # 570 "drivers/tty/serial/sc16is7xx.c"
---
> # 575 "drivers/tty/serial/sc16is7xx.c"
63180,63181c63171,63172
<          (1 << 4),
<          (1 << 4));
---
>          ((((1UL))) << (4)),
>          ((((1UL))) << (4)));
63186,63187c63177,63178
<          (1 << 7),
<          prescaler == 1 ? 0 : (1 << 7));
---
>          ((((1UL))) << (7)),
>          prescaler == 1 ? 0 : ((((1UL))) << (7)));
63192c63183
<         (1 << 7));
---
>         ((((1UL))) << (7)));
63227c63218
<    if (!(lsr & (1 << 7)))
---
>    if (!(lsr & ((((1UL))) << (7))))
63240c63231
<   lsr &= 0x1E;
---
>   lsr &= (((((1UL))) << (1)) | ((((1UL))) << (2)) | ((((1UL))) << (3)) | ((((1UL))) << (4)));
63246c63237
<    if (lsr & (1 << 4)) {
---
>    if (lsr & ((((1UL))) << (4))) {
63250c63241
<    } else if (lsr & (1 << 2))
---
>    } else if (lsr & ((((1UL))) << (2)))
63252c63243
<    else if (lsr & (1 << 3))
---
>    else if (lsr & ((((1UL))) << (3)))
63254c63245
<    else if (lsr & (1 << 1))
---
>    else if (lsr & ((((1UL))) << (1)))
63258c63249
<    if (lsr & (1 << 4))
---
>    if (lsr & ((((1UL))) << (4)))
63260c63251
<    else if (lsr & (1 << 2))
---
>    else if (lsr & ((((1UL))) << (2)))
63262c63253
<    else if (lsr & (1 << 3))
---
>    else if (lsr & ((((1UL))) << (3)))
63264c63255
<    else if (lsr & (1 << 1))
---
>    else if (lsr & ((((1UL))) << (1)))
63276c63267
<    uart_insert_char(port, lsr, (1 << 1), ch,
---
>    uart_insert_char(port, lsr, ((((1UL))) << (1)), ch,
63325c63316
<   sc16is7xx_ier_set(port, (1 << 1));
---
>   sc16is7xx_ier_set(port, ((((1UL))) << (1)));
63334,63337c63325,63328
<  mctrl |= (msr & (1 << 4)) ? 0x020 : 0;
<  mctrl |= (msr & (1 << 5)) ? 0x100 : 0;
<  mctrl |= (msr & (1 << 7)) ? 0x040 : 0;
<  mctrl |= (msr & (1 << 6)) ? 0x080 : 0;
---
>  mctrl |= (msr & ((((1UL))) << (4))) ? 0x020 : 0;
>  mctrl |= (msr & ((((1UL))) << (5))) ? 0x100 : 0;
>  mctrl |= (msr & ((((1UL))) << (7))) ? 0x040 : 0;
>  mctrl |= (msr & ((((1UL))) << (6))) ? 0x080 : 0;
63381c63372
<  if (iir & (1 << 0)) {
---
>  if (iir & 0x01) {
63386c63377
<  iir &= 0x3e;
---
>  iir &= ((((int)(sizeof(struct { int:(-!!(__builtin_choose_expr( (sizeof(int) == sizeof(*(8 ? ((void *)((long)((1) > (5)) * 0l)) : (int *)8))), (1) > (5), 0))); })))) + (((~((0UL))) - (((1UL)) << (1)) + 1) & (~((0UL)) >> (64 - 1 - (5)))));
63394c63385
< # 808 "drivers/tty/serial/sc16is7xx.c"
---
> # 813 "drivers/tty/serial/sc16is7xx.c"
63456,63457c63447,63448
<  const u32 mask = (1 << 4) |
<     (1 << 5);
---
>  const u32 mask = ((((1UL))) << (4)) |
>     ((((1UL))) << (5));
63464c63455
<   efcr |= (1 << 4);
---
>   efcr |= ((((1UL))) << (4));
63467c63458
<    efcr |= (1 << 5);
---
>    efcr |= ((((1UL))) << (5));
63485c63476
<  if (config.flags & (1 << 0)) {
---
>  if (config.flags & ((((1UL))) << (0))) {
63490c63481
<    mcr |= (1 << 1);
---
>    mcr |= ((((1UL))) << (1));
63493c63484
<    mcr |= (1 << 0);
---
>    mcr |= ((((1UL))) << (0));
63496c63487
<    mcr |= (1 << 4);
---
>    mcr |= ((((1UL))) << (4));
63498,63500c63489,63491
<           (1 << 1) |
<           (1 << 0) |
<           (1 << 4),
---
>           ((((1UL))) << (1)) |
>           ((((1UL))) << (0)) |
>           ((((1UL))) << (4)),
63504c63495
<  if (config.flags & (1 << 1))
---
>  if (config.flags & ((((1UL))) << (1)))
63508c63499
<  if (config.flags & (1 << 2))
---
>  if (config.flags & ((((1UL))) << (2)))
63554c63545
<  sc16is7xx_ier_clear(port, (1 << 0));
---
>  sc16is7xx_ier_clear(port, ((((1UL))) << (0)));
63563c63554
<  sc16is7xx_ier_set(port, (1 << 0));
---
>  sc16is7xx_ier_set(port, ((((1UL))) << (0)));
63573c63564
<  return (lsr & (1 << 6)) ? 0x01 : 0;
---
>  return (lsr & ((((1UL))) << (6))) ? 0x01 : 0;
63589c63580
<  one->config.flags |= (1 << 0);
---
>  one->config.flags |= ((((1UL))) << (0));
63596,63597c63587,63588
<          (1 << 6),
<          break_state ? (1 << 6) : 0);
---
>          ((((1UL))) << (6)),
>          break_state ? ((((1UL))) << (6)) : 0);
63637c63628
<   lcr |= (1 << 3);
---
>   lcr |= ((((1UL))) << (3));
63639c63630
<    lcr |= (1 << 4);
---
>    lcr |= ((((1UL))) << (4));
63644c63635
<   lcr |= (1 << 2);
---
>   lcr |= ((((1UL))) << (2));
63647c63638
<  port->read_status_mask = (1 << 1);
---
>  port->read_status_mask = ((((1UL))) << (1));
63649,63650c63640,63641
<   port->read_status_mask |= (1 << 2) |
<        (1 << 3);
---
>   port->read_status_mask |= ((((1UL))) << (2)) |
>        ((((1UL))) << (3));
63652c63643
<   port->read_status_mask |= (1 << 4);
---
>   port->read_status_mask |= ((((1UL))) << (4));
63657c63648
<   port->ignore_status_mask |= (1 << 4);
---
>   port->ignore_status_mask |= ((((1UL))) << (4));
63659c63650
<   port->ignore_status_mask |= 0x1E;
---
>   port->ignore_status_mask |= (((((1UL))) << (1)) | ((((1UL))) << (2)) | ((((1UL))) << (3)) | ((((1UL))) << (4)));
63664,63665c63655,63656
<   flow |= (1 << 7) |
<    (1 << 6);
---
>   flow |= ((((1UL))) << (7)) |
>    ((((1UL))) << (6));
63669c63660
<   flow |= (1 << 3);
---
>   flow |= ((((1UL))) << (3));
63671c63662
<   flow |= (1 << 1);
---
>   flow |= ((((1UL))) << (1));
63681c63672
<          ((1 << 6) | (1 << 7) | (1 << 5) | (1 << 3) | (1 << 2) | (1 << 1) | (1 << 0)), flow);
---
>          (((((1UL))) << (6)) | ((((1UL))) << (7)) | ((((1UL))) << (5)) | ((((1UL))) << (3)) | ((((1UL))) << (2)) | ((((1UL))) << (1)) | ((((1UL))) << (0))), flow);
63719c63710
<  one->config.flags |= (1 << 2);
---
>  one->config.flags |= ((((1UL))) << (2));
63734c63725
<  val = (1 << 1) | (1 << 2);
---
>  val = ((((1UL))) << (1)) | ((((1UL))) << (2));
63738c63729
<         (1 << 0));
---
>         ((((1UL))) << (0)));
63748,63749c63739,63740
<          (1 << 4),
<          (1 << 4));
---
>          ((((1UL))) << (4)),
>          ((((1UL))) << (4)));
63753,63754c63744,63745
<          (1 << 2),
<          (1 << 2));
---
>          ((((1UL))) << (2)),
>          ((((1UL))) << (2)));
63770c63761
<          (1 << 6),
---
>          ((((1UL))) << (6)),
63772c63763
<     (1 << 6) : 0);
---
>     ((((1UL))) << (6)) : 0);
63776,63777c63767,63768
<          (1 << 1) |
<          (1 << 2),
---
>          ((((1UL))) << (1)) |
>          ((((1UL))) << (2)),
63781,63782c63772,63773
<  val = (1 << 0) | (1 << 7) |
<        (1 << 3);
---
>  val = ((((1UL))) << (0)) | ((((1UL))) << (7)) |
>        ((((1UL))) << (3));
63804,63807c63795,63798
<          (1 << 1) |
<          (1 << 2),
<          (1 << 1) |
<          (1 << 2));
---
>          ((((1UL))) << (1)) |
>          ((((1UL))) << (2)),
>          ((((1UL))) << (1)) |
>          ((((1UL))) << (2)));
63876c63867
< # 1400 "drivers/tty/serial/sc16is7xx.c"
---
> # 1405 "drivers/tty/serial/sc16is7xx.c"
63926c63917
<    s->mctrl_mask |= (1 << 1);
---
>    s->mctrl_mask |= ((((1UL))) << (1));
63928c63919
<    s->mctrl_mask |= (1 << 2);
---
>    s->mctrl_mask |= ((((1UL))) << (2));
63935,63936c63926,63927
<    (1 << 1) |
<    (1 << 2), s->mctrl_mask);
---
>    ((((1UL))) << (1)) |
>    ((((1UL))) << (2)), s->mctrl_mask);
63960c63951
< # 1493 "drivers/tty/serial/sc16is7xx.c"
---
> # 1498 "drivers/tty/serial/sc16is7xx.c"
64009c64000
<        (1 << 3));
---
>        ((((1UL))) << (3)));
64056,64057c64047,64048
<          (1 << 1) |
<          (1 << 2));
---
>          ((((1UL))) << (1)) |
>          ((((1UL))) << (2)));
64079c64070
<          (1 << 4));
---
>          ((((1UL))) << (4)));
64095c64086
< # 1640 "drivers/tty/serial/sc16is7xx.c"
---
> # 1645 "drivers/tty/serial/sc16is7xx.c"
64188c64179
<   ({ int __ret_warn_on = !!(true); if (__builtin_expect(!!(__ret_warn_on), 0)) do { __auto_type __flags = (1 << 0)|(((9) << 8)); ({ asm volatile("406" ": nop\n\t" ".pushsection .discard.instr_begin\n\t" ".long " "406" "b - .\n\t" ".popsection\n\t" : : "i" (406)); }); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - ." "\t# bug_entry::bug_addr\n" "\t" ".long " "%c0" " - ." "\t# bug_entry::file\n" "\t.word %c1" "\t# bug_entry::line\n" "\t.word %c2" "\t# bug_entry::flags\n" "\t.org 2b+%c3\n" ".popsection\n" "998:\n\t" ".pushsection .discard.reachable\n\t" ".long 998b\n\t" ".popsection\n\t" : : "i" ("drivers/tty/serial/sc16is7xx.c"), "i" (1732), "i" (__flags), "i" (sizeof(struct bug_entry))); } while (0); ({ asm volatile("407" ": nop\n\t" ".pushsection .discard.instr_end\n\t" ".long " "407" "b - .\n\t" ".popsection\n\t" : : "i" (407)); }); } while (0); __builtin_expect(!!(__ret_warn_on), 0); });
---
>   ({ int __ret_warn_on = !!(true); if (__builtin_expect(!!(__ret_warn_on), 0)) do { __auto_type __flags = (1 << 0)|(((9) << 8)); ({ asm volatile("406" ": nop\n\t" ".pushsection .discard.instr_begin\n\t" ".long " "406" "b - .\n\t" ".popsection\n\t" : : "i" (406)); }); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - ." "\t# bug_entry::bug_addr\n" "\t" ".long " "%c0" " - ." "\t# bug_entry::file\n" "\t.word %c1" "\t# bug_entry::line\n" "\t.word %c2" "\t# bug_entry::flags\n" "\t.org 2b+%c3\n" ".popsection\n" "998:\n\t" ".pushsection .discard.reachable\n\t" ".long 998b\n\t" ".popsection\n\t" : : "i" ("drivers/tty/serial/sc16is7xx.c"), "i" (1737), "i" (__flags), "i" (sizeof(struct bug_entry))); } while (0); ({ asm volatile("407" ": nop\n\t" ".pushsection .discard.instr_end\n\t" ".long " "407" "b - .\n\t" ".popsection\n\t" : : "i" (407)); }); } while (0); __builtin_expect(!!(__ret_warn_on), 0); });
64205c64196
< static void * __attribute__((__used__)) __attribute__((__section__(".discard.addressable"))) __UNIQUE_ID___addressable_sc16is7xx_init411 = (void *)(uintptr_t)&sc16is7xx_init; asm(".section	\"" ".initcall6" ".init" "\", \"a\"		\n" "__initcall__kmod_sc16is7xx__410_1749_sc16is7xx_init6" ":			\n" ".long	" "sc16is7xx_init" " - .	\n" ".previous					\n"); _Static_assert(__builtin_types_compatible_p(typeof(initcall_t), typeof(&sc16is7xx_init)), "__same_type(initcall_t, &sc16is7xx_init)");;;
---
> static void * __attribute__((__used__)) __attribute__((__section__(".discard.addressable"))) __UNIQUE_ID___addressable_sc16is7xx_init411 = (void *)(uintptr_t)&sc16is7xx_init; asm(".section	\"" ".initcall6" ".init" "\", \"a\"		\n" "__initcall__kmod_sc16is7xx__410_1754_sc16is7xx_init6" ":			\n" ".long	" "sc16is7xx_init" " - .	\n" ".previous					\n"); _Static_assert(__builtin_types_compatible_p(typeof(initcall_t), typeof(&sc16is7xx_init)), "__same_type(initcall_t, &sc16is7xx_init)");;;


-- 
Pozdrawiam/With kind regards,
Lech Perczak

Sr. Software Engineer
Camlin Technologies Poland Limited Sp. z o.o.

Re: [PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[Jiri Slaby]

On 27. 08. 24, 17:38, Lech Perczak wrote:
> Only the single expansion of GENMASK proves very hard to analyze in the
> diff - so I double-checked with a calculator,
> though all BIT() expansions do match, as does the updated definition.
> of SC16IS7XX_LSR_BRK_ERROR_MASK.

You can diff also the generated .o fed through objdump -d (generated .s 
usually contains line numbers).

regards,
-- 
js
suse labs

Re: [PATCH v4 0/3] serial: sc16is7xx: cosmetic cleanup

[Lech Perczak]

Hello Jiri,

W dniu 28.08.2024 o 06:35, Jiri Slaby pisze:
> On 27. 08. 24, 17:38, Lech Perczak wrote:
>> Only the single expansion of GENMASK proves very hard to analyze in the
>> diff - so I double-checked with a calculator,
>> though all BIT() expansions do match, as does the updated definition.
>> of SC16IS7XX_LSR_BRK_ERROR_MASK.
> 
> You can diff also the generated .o fed through objdump -d (generated .s 
> usually contains line numbers).

I see that Greg already got the series into tty-next, but for the sake of completeness,
I did the check anyway.
This also came to my mind after I posted diff of preprocessed source ;-)

The diff is way, way smaller, but functionality is the same.
On amd64, the sign extension used in BIT() is visible:

--- sc16is7xx.c.asm.1	2024-09-04 16:06:02.523038659 +0200
+++ sc16is7xx.c.asm.2	2024-09-04 16:05:42.103938886 +0200
@@ -289,17 +289,16 @@
      1f9:	c7 44 24 04 00 00 00 	movl   $0x0,0x4(%rsp)
      200:	00 
      201:	e8 00 00 00 00       	call   206 <sc16is7xx_tx_empty+0x36>
-     206:	8b 44 24 04          	mov    0x4(%rsp),%eax
-     20a:	c1 e8 06             	shr    $0x6,%eax
-     20d:	83 e0 01             	and    $0x1,%eax
-     210:	48 8b 54 24 08       	mov    0x8(%rsp),%rdx
-     215:	65 48 2b 14 25 28 00 	sub    %gs:0x28,%rdx
-     21c:	00 00 
-     21e:	75 09                	jne    229 <sc16is7xx_tx_empty+0x59>
-     220:	48 83 c4 10          	add    $0x10,%rsp
-     224:	e9 00 00 00 00       	jmp    229 <sc16is7xx_tx_empty+0x59>
-     229:	e8 00 00 00 00       	call   22e <sc16is7xx_tx_empty+0x5e>
-     22e:	66 90                	xchg   %ax,%ax
+     206:	0f b6 44 24 04       	movzbl 0x4(%rsp),%eax
+     20b:	48 c1 e8 06          	shr    $0x6,%rax
+     20f:	83 e0 01             	and    $0x1,%eax
+     212:	48 8b 54 24 08       	mov    0x8(%rsp),%rdx
+     217:	65 48 2b 14 25 28 00 	sub    %gs:0x28,%rdx
+     21e:	00 00 
+     220:	75 09                	jne    22b <sc16is7xx_tx_empty+0x5b>
+     222:	48 83 c4 10          	add    $0x10,%rsp
+     226:	e9 00 00 00 00       	jmp    22b <sc16is7xx_tx_empty+0x5b>
+     22b:	e8 00 00 00 00       	call   230 <sc16is7xx_tx_empty+0x60>
      230:	90                   	nop
      231:	90                   	nop
      232:	90                   	nop

> 
> regards,

-- 
Pozdrawiam/With kind regards,
Lech Perczak

Sr. Software Engineer
Camlin Technologies Poland Limited Sp. z o.o.