linux.git/drivers/tty/serial/pic32_uart.c, branch v5.19 Linux kernel source tree serial: pic32: restore disabled irqs in pic32_uart_startup() 2022-05-05T20:39:20+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:22+00:00 dfb9afb6c0e7af7c381c88d72d6d9c64268c160f pic32_uart_startup() disables interrupts by local_irq_save(). But the function never enables them. The serial core only holds a mutex, so irqs are not restored. So how could this driver work? This irq handling was already present in the driver's initial commit 157b9394709ed (serial: pic32_uart: Add PIC32 UART driver). So is it a candidate for removal? Anyone has a contact to the author: Andrei Pistirica (I believe the one below -- @microchip.com -- will bounce)? Or to someone else @microchip.com? Cc: Andrei Pistirica <andrei.pistirica@microchip.com> Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-12-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
pic32_uart_startup() disables interrupts by local_irq_save(). But the
function never enables them. The serial core only holds a mutex, so irqs
are not restored.

So how could this driver work? This irq handling was already present in
the driver's initial commit 157b9394709ed (serial: pic32_uart: Add PIC32
UART driver).

So is it a candidate for removal? Anyone has a contact to the author:
Andrei Pistirica (I believe the one below -- @microchip.com -- will
bounce)? Or to someone else @microchip.com?

Cc: Andrei Pistirica <andrei.pistirica@microchip.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-12-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: free up irq names correctly 2022-05-05T20:39:19+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:21+00:00 fe36fa18ca77ca3ca9f90aab6cf39031416e432b struct pic32_sport contains built-up names for irqs. These are freed only in error path of pic32_uart_startup(). And even there, the freeing happens before free_irq(). So fix this by: * moving frees after free_irq(), and * add frees to pic32_uart_shutdown() -- the opposite of pic32_uart_startup(). Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-11-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
struct pic32_sport contains built-up names for irqs. These are freed
only in error path of pic32_uart_startup(). And even there, the freeing
happens before free_irq().

So fix this by:
* moving frees after free_irq(), and
* add frees to pic32_uart_shutdown() -- the opposite of
  pic32_uart_startup().

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-11-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: don't zero members of kzalloc-ated structure 2022-05-05T20:39:19+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:20+00:00 28dc563339b181461494918d5d155e3ebc9a87ec struct pic32_sport (sport) has just been kzallocated. So there is no need to zero its member (sport->port) now. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-10-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
struct pic32_sport (sport) has just been kzallocated. So there is no
need to zero its member (sport->port) now.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-10-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: don't assign pic32_sport::cts_gpio twice 2022-05-05T20:39:19+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:19+00:00 0ed55be47c99cb156f96ad95649760d474d9c56c sport->cts_gpio is first assigned -EINVAL and few lines below using of_get_named_gpio(). Remove the first (useless) assignment. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-9-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
sport->cts_gpio is first assigned -EINVAL and few lines below using
of_get_named_gpio(). Remove the first (useless) assignment.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-9-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: convert to_pic32_sport() to an inline 2022-05-05T20:39:18+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:18+00:00 412314720aec9226e7e423395d27ca17d5dd3b24 'c' is not in wrapped in parentheses in the to_pic32_sport() macro, so it might be problematic wrt macro expansion. Using an inline is always safer in these cases. Both type-wise and macro-expansion-wise. So switch the macro to an inline. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-8-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
'c' is not in wrapped in parentheses in the to_pic32_sport() macro, so
it might be problematic wrt macro expansion. Using an inline is always
safer in these cases. Both type-wise and macro-expansion-wise. So switch
the macro to an inline.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-8-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: remove pic32_get_port() macro 2022-05-05T20:39:18+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:17+00:00 343f23cfc22b1d1577289db694f6896d1ac26594 It's just &sport->port. First, sport was not in parenthesis, so macro expansion could be an issue. Second, it's so simple, that we can expand the macro and make the code really straightforward. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-7-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
It's just &sport->port. First, sport was not in parenthesis, so macro
expansion could be an issue. Second, it's so simple, that we can expand
the macro and make the code really straightforward.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-7-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: simplify pic32_sport::enable_tx_irq handling 2022-05-05T20:39:17+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:16+00:00 e8616bd0e9f24a9265fe5db1e7963185514b445a Make it a bool, so use true+false. And remove the wrap-around macro -- i.e. access the member directly. It makes the code more obvious. BTW the macro did not have 'sport' in parentheses, so it was potentially problematic wrt expansion. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-6-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Make it a bool, so use true+false. And remove the wrap-around macro --
i.e. access the member directly.

It makes the code more obvious.

BTW the macro did not have 'sport' in parentheses, so it was potentially
problematic wrt expansion.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-6-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: simplify clk handling 2022-05-05T20:39:17+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:15+00:00 bb2cff419d3253aac12e7513be2b0fd7ee11ad78 struct pic32_sport::ref_clk is only set, but not read. That means we can remove it. And when we do so, pic32_enable_clock() and pic32_disable_clock() are simple wrappers around clk_prepare_enable() and clk_disable_unprepare() respectively. So we can remove the former two from the code and replace it by the latter two. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-5-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
struct pic32_sport::ref_clk is only set, but not read. That means we can
remove it. And when we do so, pic32_enable_clock() and
pic32_disable_clock() are simple wrappers around clk_prepare_enable()
and clk_disable_unprepare() respectively. So we can remove the former
two from the code and replace it by the latter two.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-5-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: remove constants from struct pic32_sport 2022-05-05T20:39:17+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:14+00:00 08f643e0224222aa089265a72690187898cac3e7 All the irqflags_* in struct pic32_sport are set to IRQF_NO_THREAD and never updated. So remove pic32_sport::irqflags_* and use the flag directly. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-4-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
All the irqflags_* in struct pic32_sport are set to IRQF_NO_THREAD and
never updated. So remove pic32_sport::irqflags_* and use the flag
directly.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-4-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
serial: pic32: move header content to .c 2022-05-05T20:39:16+00:00 Jiri Slaby jslaby@suse.cz 2022-05-03T06:31:13+00:00 29574d0ded40f1834f4a4a054d213fdc0642d60a There is no point keeping the header content separated. So move the content to the appropriate source file. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Link: https://lore.kernel.org/r/20220503063122.20957-3-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
There is no point keeping the header content separated. So move the
content to the appropriate source file.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-3-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>