diff mbox series

[V3,1/6] tty: serial: meson: Drop the legacy compatible strings and clock code

Message ID 20211230102110.3861-2-yu.tu@amlogic.com (mailing list archive)
State New, archived
Headers show
Series the UART driver compatible with the Amlogic Meson | expand

Commit Message

Yu Tu Dec. 30, 2021, 10:21 a.m. UTC
All mainline .dts files have been using the stable UART since Linux
4.16. Drop the legacy compatible strings and related clock code.

Signed-off-by: Yu Tu <yu.tu@amlogic.com>
---
 drivers/tty/serial/meson_uart.c | 34 ++-------------------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

Comments

Martin Blumenstingl Dec. 30, 2021, 10:22 p.m. UTC | #1
On Thu, Dec 30, 2021 at 11:21 AM Yu Tu <yu.tu@amlogic.com> wrote:
>
> All mainline .dts files have been using the stable UART since Linux
> 4.16. Drop the legacy compatible strings and related clock code.
>
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
I have just realized that this cannot be dropped until my series "ARM:
dts: meson: fix UART device-tree schema validation" [0] is merged

[...]
> -/* Legacy bindings, should be removed when no more used */
> -OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
> -                   meson_serial_early_console_setup);
This part is still needed as long as above series is not merged yet.
If we remove this then earlycon will stop working on the 32-bit SoCs
unless [0] is merged.

All other code below - except the of_device_id entry - can still be
removed since meson8.dtsi and meson8b.dtsi are using the non-legacy
clocks already.

Sorry for only noticing this now.


Best regards,
Martin


[0] https://patchwork.kernel.org/project/linux-amlogic/cover/20211227180026.4068352-1-martin.blumenstingl@googlemail.com/
Yu Tu Dec. 31, 2021, 10:27 a.m. UTC | #2
Hi Martin,
	Thank you very much for your reply.

On 2021/12/31 6:22, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> On Thu, Dec 30, 2021 at 11:21 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> All mainline .dts files have been using the stable UART since Linux
>> 4.16. Drop the legacy compatible strings and related clock code.
>>
>> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> I have just realized that this cannot be dropped until my series "ARM:
> dts: meson: fix UART device-tree schema validation" [0] is merged
> 
> [...]
>> -/* Legacy bindings, should be removed when no more used */
>> -OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
>> -                   meson_serial_early_console_setup);
> This part is still needed as long as above series is not merged yet.
> If we remove this then earlycon will stop working on the 32-bit SoCs
> unless [0] is merged.
> 
> All other code below - except the of_device_id entry - can still be
> removed since meson8.dtsi and meson8b.dtsi are using the non-legacy
> clocks already.
> 
> Sorry for only noticing this now.
> 
I will add it back in the next patch and delete it after your submission 
is merged.
> 
> Best regards,
> Martin
> 
> 
> [0] https://patchwork.kernel.org/project/linux-amlogic/cover/20211227180026.4068352-1-martin.blumenstingl@googlemail.com/
>
Martin Blumenstingl Dec. 31, 2021, 3:35 p.m. UTC | #3
On Fri, Dec 31, 2021 at 11:27 AM Yu Tu <yu.tu@amlogic.com> wrote:
[...]
> >> -/* Legacy bindings, should be removed when no more used */
> >> -OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
> >> -                   meson_serial_early_console_setup);
> > This part is still needed as long as above series is not merged yet.
> > If we remove this then earlycon will stop working on the 32-bit SoCs
> > unless [0] is merged.
> >
> > All other code below - except the of_device_id entry - can still be
> > removed since meson8.dtsi and meson8b.dtsi are using the non-legacy
> > clocks already.
> >
> > Sorry for only noticing this now.
> >
> I will add it back in the next patch and delete it after your submission
> is merged.
I have just seen that Greg has already added this patch to the tty-next tree [1]
In this case there's nothing to do on your end - I'll simply ask Neil
to also queue my 32-bit SoC UART .dts fixes [0] for 5.17


Best regards,
Martin


[0] https://patchwork.kernel.org/project/linux-amlogic/cover/20211227180026.4068352-1-martin.blumenstingl@googlemail.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=ad234e2bac274a43c9fa540bde8cd9f0c627b71f
Neil Armstrong Jan. 3, 2022, 2:59 p.m. UTC | #4
Hi Greg,

Martin just saw this patch was applied, but the serie wasn't reviewed enough and it will break
earlycon support on the ARMv7 Amlogic SoCs fore 5.17.

Anyway, I'll push the corresponding DT fixes for 5.17-rc1.

Would it be possible we also receive the notification when those patches are applied ?
Maybe a MAINTAINERS entry is missing so we can receive them ?

It would help me track those TTY and USB patches more easily.

Thanks !

Neil

On 31/12/2021 16:35, Martin Blumenstingl wrote:
> On Fri, Dec 31, 2021 at 11:27 AM Yu Tu <yu.tu@amlogic.com> wrote:
> [...]
>>>> -/* Legacy bindings, should be removed when no more used */
>>>> -OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
>>>> -                   meson_serial_early_console_setup);
>>> This part is still needed as long as above series is not merged yet.
>>> If we remove this then earlycon will stop working on the 32-bit SoCs
>>> unless [0] is merged.
>>>
>>> All other code below - except the of_device_id entry - can still be
>>> removed since meson8.dtsi and meson8b.dtsi are using the non-legacy
>>> clocks already.
>>>
>>> Sorry for only noticing this now.
>>>
>> I will add it back in the next patch and delete it after your submission
>> is merged.
> I have just seen that Greg has already added this patch to the tty-next tree [1]
> In this case there's nothing to do on your end - I'll simply ask Neil
> to also queue my 32-bit SoC UART .dts fixes [0] for 5.17
> 
> 
> Best regards,
> Martin
> 
> 
> [0] https://patchwork.kernel.org/project/linux-amlogic/cover/20211227180026.4068352-1-martin.blumenstingl@googlemail.com/
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=ad234e2bac274a43c9fa540bde8cd9f0c627b71f
>
Greg Kroah-Hartman Jan. 3, 2022, 3:19 p.m. UTC | #5
A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Mon, Jan 03, 2022 at 03:59:33PM +0100, Neil Armstrong wrote:
> Hi Greg,
> 
> Martin just saw this patch was applied, but the serie wasn't reviewed enough and it will break
> earlycon support on the ARMv7 Amlogic SoCs fore 5.17.

Ok, what should I revert?

> Anyway, I'll push the corresponding DT fixes for 5.17-rc1.

How did we get out of sync here?

> Would it be possible we also receive the notification when those patches are applied ?
> Maybe a MAINTAINERS entry is missing so we can receive them ?

That would be good, so that people can review the patches.  Otherwise I
have to just guess :)

> It would help me track those TTY and USB patches more easily.

I recommend MAINTAINERS entries for drivers that are not listed and that
you care about seeing the changes for.

thanks,

greg k-h
Neil Armstrong Jan. 3, 2022, 3:29 p.m. UTC | #6
Hi Greg,
On 03/01/2022 16:19, Greg Kroah-Hartman wrote:
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> http://daringfireball.net/2007/07/on_to>
> On Mon, Jan 03, 2022 at 03:59:33PM +0100, Neil Armstrong wrote:
>> Hi Greg,
>>
>> Martin just saw this patch was applied, but the serie wasn't reviewed enough and it will break
>> earlycon support on the ARMv7 Amlogic SoCs fore 5.17.
> 
> Ok, what should I revert?

None, we have a fix in the pipe

> 
>> Anyway, I'll push the corresponding DT fixes for 5.17-rc1.
> 
> How did we get out of sync here?

The serie wasn't fully reviewed, and I was out of office when it was applied.

> 
>> Would it be possible we also receive the notification when those patches are applied ?
>> Maybe a MAINTAINERS entry is missing so we can receive them ?
> 
> That would be good, so that people can review the patches.  Otherwise I
> have to just guess :)

exact, I naively thought it would be matched in the:
N:      meson
entry but it seems an proper entry for drivers/tty/serial/meson_uart.c is needed.

> 
>> It would help me track those TTY and USB patches more easily.
> 
> I recommend MAINTAINERS entries for drivers that are not listed and that
> you care about seeing the changes for.

This is why we have a regex to match these.

I can submit a patch to have a proper entry if the regex is not enough/appropriate.

BTW can you point us how are selected the recipients of the notification messages you send ?

Thanks,
Neil

> 
> thanks,
> 
> greg k-h
>
Greg Kroah-Hartman Jan. 3, 2022, 4:29 p.m. UTC | #7
On Mon, Jan 03, 2022 at 04:29:56PM +0100, Neil Armstrong wrote:
> Hi Greg,
> On 03/01/2022 16:19, Greg Kroah-Hartman wrote:
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> > 
> > A: No.
> > Q: Should I include quotations after my reply?
> > 
> > http://daringfireball.net/2007/07/on_to>
> > On Mon, Jan 03, 2022 at 03:59:33PM +0100, Neil Armstrong wrote:
> >> Hi Greg,
> >>
> >> Martin just saw this patch was applied, but the serie wasn't reviewed enough and it will break
> >> earlycon support on the ARMv7 Amlogic SoCs fore 5.17.
> > 
> > Ok, what should I revert?
> 
> None, we have a fix in the pipe
> 
> > 
> >> Anyway, I'll push the corresponding DT fixes for 5.17-rc1.
> > 
> > How did we get out of sync here?
> 
> The serie wasn't fully reviewed, and I was out of office when it was applied.
> 
> > 
> >> Would it be possible we also receive the notification when those patches are applied ?
> >> Maybe a MAINTAINERS entry is missing so we can receive them ?
> > 
> > That would be good, so that people can review the patches.  Otherwise I
> > have to just guess :)
> 
> exact, I naively thought it would be matched in the:
> N:      meson
> entry but it seems an proper entry for drivers/tty/serial/meson_uart.c is needed.

Try it, does that work when running get_maintainer.pl on this patch?

> >> It would help me track those TTY and USB patches more easily.
> > 
> > I recommend MAINTAINERS entries for drivers that are not listed and that
> > you care about seeing the changes for.
> 
> This is why we have a regex to match these.

Great, the submitter should have used that.

> I can submit a patch to have a proper entry if the regex is not enough/appropriate.

Whatever makes the tools work is fine with me.

> BTW can you point us how are selected the recipients of the notification messages you send ?

For when I apply a patch, everyone on the ack/signed-off-by/reviewed-by
list gets a response.  I do not hit mailing lists with the notification
as that's just too much noise.

Been doing it this way for well over a decade now, nothing new here :)

thanks,

greg k-h
Neil Armstrong Jan. 3, 2022, 4:35 p.m. UTC | #8
On 03/01/2022 17:29, Greg Kroah-Hartman wrote:
> On Mon, Jan 03, 2022 at 04:29:56PM +0100, Neil Armstrong wrote:
>>>> Would it be possible we also receive the notification when those patches are applied ?
>>>> Maybe a MAINTAINERS entry is missing so we can receive them ?
>>>
>>> That would be good, so that people can review the patches.  Otherwise I
>>> have to just guess :)
>>
>> exact, I naively thought it would be matched in the:
>> N:      meson
>> entry but it seems an proper entry for drivers/tty/serial/meson_uart.c is needed.
> 
> Try it, does that work when running get_maintainer.pl on this patch?

Yes it does work, so no problem here

> 
>>>> It would help me track those TTY and USB patches more easily.
>>>
>>> I recommend MAINTAINERS entries for drivers that are not listed and that
>>> you care about seeing the changes for.
>>
>> This is why we have a regex to match these.
> 
> Great, the submitter should have used that.
> 
>> I can submit a patch to have a proper entry if the regex is not enough/appropriate.
> 
> Whatever makes the tools work is fine with me.
> 
>> BTW can you point us how are selected the recipients of the notification messages you send ?
> 
> For when I apply a patch, everyone on the ack/signed-off-by/reviewed-by
> list gets a response.  I do not hit mailing lists with the notification
> as that's just too much noise.
> 
> Been doing it this way for well over a decade now, nothing new here :)

Ok right, no problem, other maintainers (e.g: net, sound) and default b4 behavior is to
send notification to same recipient as original patch.

It it fits everyone for a decade, no need to change ! We have very low patches for tty & usb anyway

Neil

> 
> thanks,
> 
> greg k-h
>
diff mbox series

Patch

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index d2c08b760f83..c9a37602ffd0 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -625,10 +625,7 @@  meson_serial_early_console_setup(struct earlycon_device *device, const char *opt
 	device->con->write = meson_serial_early_console_write;
 	return 0;
 }
-/* Legacy bindings, should be removed when no more used */
-OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
-		    meson_serial_early_console_setup);
-/* Stable bindings */
+
 OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
 		    meson_serial_early_console_setup);
 
@@ -668,25 +665,6 @@  static inline struct clk *meson_uart_probe_clock(struct device *dev,
 	return clk;
 }
 
-/*
- * This function gets clocks in the legacy non-stable DT bindings.
- * This code will be remove once all the platforms switch to the
- * new DT bindings.
- */
-static int meson_uart_probe_clocks_legacy(struct platform_device *pdev,
-					  struct uart_port *port)
-{
-	struct clk *clk = NULL;
-
-	clk = meson_uart_probe_clock(&pdev->dev, NULL);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
-	port->uartclk = clk_get_rate(clk);
-
-	return 0;
-}
-
 static int meson_uart_probe_clocks(struct platform_device *pdev,
 				   struct uart_port *port)
 {
@@ -750,12 +728,7 @@  static int meson_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
-	/* Use legacy way until all platforms switch to new bindings */
-	if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
-		ret = meson_uart_probe_clocks_legacy(pdev, port);
-	else
-		ret = meson_uart_probe_clocks(pdev, port);
-
+	ret = meson_uart_probe_clocks(pdev, port);
 	if (ret)
 		return ret;
 
@@ -800,9 +773,6 @@  static int meson_uart_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id meson_uart_dt_match[] = {
-	/* Legacy bindings, should be removed when no more used */
-	{ .compatible = "amlogic,meson-uart" },
-	/* Stable bindings */
 	{ .compatible = "amlogic,meson6-uart" },
 	{ .compatible = "amlogic,meson8-uart" },
 	{ .compatible = "amlogic,meson8b-uart" },