* [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device
@ 2021-07-21 11:33 Dongliang Mu
2021-07-21 12:00 ` Johan Hovold
0 siblings, 1 reply; 5+ messages in thread
From: Dongliang Mu @ 2021-07-21 11:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Dongliang Mu, stable, Greg Kroah-Hartman, linux-kernel
The pairwise api invocation of tty_port_register_device should be
tty_port_unregister_device, other than tty_unregister_device.
Fixes: a6afd9f3e819 ("tty: move a number of tty drivers from drivers/char/ to drivers/tty/")
Cc: stable@vger.kernel.org
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
drivers/tty/nozomi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index 0c80f25c8c3d..08bdd82f60b5 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -1417,7 +1417,8 @@ static int nozomi_card_init(struct pci_dev *pdev,
err_free_tty:
for (i--; i >= 0; i--) {
- tty_unregister_device(ntty_driver, dc->index_start + i);
+ tty_port_unregister_device(&dc->port[i].port, ntty_driver,
+ dc->index_start + i);
tty_port_destroy(&dc->port[i].port);
}
free_irq(pdev->irq, dc);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device
2021-07-21 11:33 [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device Dongliang Mu
@ 2021-07-21 12:00 ` Johan Hovold
2021-07-21 12:53 ` Dongliang Mu
0 siblings, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-07-21 12:00 UTC (permalink / raw)
To: Dongliang Mu
Cc: Greg Kroah-Hartman, Jiri Slaby, stable, Greg Kroah-Hartman, linux-kernel
On Wed, Jul 21, 2021 at 07:33:04PM +0800, Dongliang Mu wrote:
> The pairwise api invocation of tty_port_register_device should be
> tty_port_unregister_device, other than tty_unregister_device.
Are you sure about that? Please explain why you think this to be the
case and why this change is needed.
> Fixes: a6afd9f3e819 ("tty: move a number of tty drivers from drivers/char/ to drivers/tty/")
Please try a little harder, that's clearly not the commit that changed
to the port registration helper.
> Cc: stable@vger.kernel.org
Why do you think this is stable material? (hint: it is not)
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> drivers/tty/nozomi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
> index 0c80f25c8c3d..08bdd82f60b5 100644
> --- a/drivers/tty/nozomi.c
> +++ b/drivers/tty/nozomi.c
> @@ -1417,7 +1417,8 @@ static int nozomi_card_init(struct pci_dev *pdev,
>
> err_free_tty:
> for (i--; i >= 0; i--) {
> - tty_unregister_device(ntty_driver, dc->index_start + i);
> + tty_port_unregister_device(&dc->port[i].port, ntty_driver,
> + dc->index_start + i);
> tty_port_destroy(&dc->port[i].port);
> }
> free_irq(pdev->irq, dc);
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device
2021-07-21 12:00 ` Johan Hovold
@ 2021-07-21 12:53 ` Dongliang Mu
2021-07-21 13:09 ` Johan Hovold
0 siblings, 1 reply; 5+ messages in thread
From: Dongliang Mu @ 2021-07-21 12:53 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Jiri Slaby, stable, Greg Kroah-Hartman, linux-kernel
On Wed, Jul 21, 2021 at 8:01 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Jul 21, 2021 at 07:33:04PM +0800, Dongliang Mu wrote:
> > The pairwise api invocation of tty_port_register_device should be
> > tty_port_unregister_device, other than tty_unregister_device.
>
> Are you sure about that? Please explain why you think this to be the
> case and why this change is needed.
I am sure about this.
1. From the implementation,
tty_port_register_device -> tty_port_register_device_attr ->
tty_port_link_device; tty_register_device_attr
tty_register_device -> tty_register_device_attr
tty_port_unregister_device -> serdev_tty_port_unregister;
tty_unregister_device
tty_unregister_device
As to the functionability, tty_port_register_device pairs with
tty_port_unregister_device; meanwhile, the same to tty_register_device
and tty_unregister_device.
2. From the function naming style,
tty_port_register_device - tty_port_unregister_device;
tty_register_device - tty_unregister_device
>
> > Fixes: a6afd9f3e819 ("tty: move a number of tty drivers from drivers/char/ to drivers/tty/")
>
> Please try a little harder, that's clearly not the commit that changed
> to the port registration helper.
>
> > Cc: stable@vger.kernel.org
>
> Why do you think this is stable material? (hint: it is not)
From the documentation, this label could make the patch automatically
go to stable tree. And stable tree is also using the incorrect api.
If I have any misunderstanding, please let me know.
>
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> > drivers/tty/nozomi.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
> > index 0c80f25c8c3d..08bdd82f60b5 100644
> > --- a/drivers/tty/nozomi.c
> > +++ b/drivers/tty/nozomi.c
> > @@ -1417,7 +1417,8 @@ static int nozomi_card_init(struct pci_dev *pdev,
> >
> > err_free_tty:
> > for (i--; i >= 0; i--) {
> > - tty_unregister_device(ntty_driver, dc->index_start + i);
> > + tty_port_unregister_device(&dc->port[i].port, ntty_driver,
> > + dc->index_start + i);
> > tty_port_destroy(&dc->port[i].port);
> > }
> > free_irq(pdev->irq, dc);
>
> Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device
2021-07-21 12:53 ` Dongliang Mu
@ 2021-07-21 13:09 ` Johan Hovold
2021-07-21 13:35 ` Dongliang Mu
0 siblings, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-07-21 13:09 UTC (permalink / raw)
To: Dongliang Mu
Cc: Greg Kroah-Hartman, Jiri Slaby, stable, Greg Kroah-Hartman, linux-kernel
On Wed, Jul 21, 2021 at 08:53:47PM +0800, Dongliang Mu wrote:
> On Wed, Jul 21, 2021 at 8:01 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Wed, Jul 21, 2021 at 07:33:04PM +0800, Dongliang Mu wrote:
> > > The pairwise api invocation of tty_port_register_device should be
> > > tty_port_unregister_device, other than tty_unregister_device.
> >
> > Are you sure about that? Please explain why you think this to be the
> > case and why this change is needed.
>
> I am sure about this.
I'm afraid you are mistaken. There is a bit of inconsistency in the API,
but it is *not* a requirement to use the port helper for deregistration
here.
> 1. From the implementation,
> tty_port_register_device -> tty_port_register_device_attr ->
> tty_port_link_device; tty_register_device_attr
> tty_register_device -> tty_register_device_attr
>
> tty_port_unregister_device -> serdev_tty_port_unregister;
> tty_unregister_device
> tty_unregister_device
> As to the functionability, tty_port_register_device pairs with
> tty_port_unregister_device; meanwhile, the same to tty_register_device
> and tty_unregister_device.
Again, this is not an explanation. Why do think it is needed? What could
possibly go wrong if you don't change the code like you propose?
> 2. From the function naming style,
>
> tty_port_register_device - tty_port_unregister_device;
> tty_register_device - tty_unregister_device
Yes, the naming suggests you should be using the port helper and it is
ok to do so, but again, it is not a requirement (unless you're using the
serdev variant).
> > > Fixes: a6afd9f3e819 ("tty: move a number of tty drivers from drivers/char/ to drivers/tty/")
> >
> > Please try a little harder, that's clearly not the commit that changed
> > to the port registration helper.
> >
> > > Cc: stable@vger.kernel.org
> >
> > Why do you think this is stable material? (hint: it is not)
>
> From the documentation, this label could make the patch automatically
> go to stable tree. And stable tree is also using the incorrect api.
No, it is not using an "incorrect api". There is nothing wrong with
current code. And it certainly does not need to be changed in stable.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device
2021-07-21 13:09 ` Johan Hovold
@ 2021-07-21 13:35 ` Dongliang Mu
0 siblings, 0 replies; 5+ messages in thread
From: Dongliang Mu @ 2021-07-21 13:35 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Jiri Slaby, stable, Greg Kroah-Hartman, linux-kernel
On Wed, Jul 21, 2021 at 9:09 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Jul 21, 2021 at 08:53:47PM +0800, Dongliang Mu wrote:
> > On Wed, Jul 21, 2021 at 8:01 PM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Wed, Jul 21, 2021 at 07:33:04PM +0800, Dongliang Mu wrote:
> > > > The pairwise api invocation of tty_port_register_device should be
> > > > tty_port_unregister_device, other than tty_unregister_device.
> > >
> > > Are you sure about that? Please explain why you think this to be the
> > > case and why this change is needed.
> >
> > I am sure about this.
>
> I'm afraid you are mistaken. There is a bit of inconsistency in the API,
> but it is *not* a requirement to use the port helper for deregistration
> here.
>
> > 1. From the implementation,
> > tty_port_register_device -> tty_port_register_device_attr ->
> > tty_port_link_device; tty_register_device_attr
> > tty_register_device -> tty_register_device_attr
> >
> > tty_port_unregister_device -> serdev_tty_port_unregister;
> > tty_unregister_device
> > tty_unregister_device
>
> > As to the functionability, tty_port_register_device pairs with
> > tty_port_unregister_device; meanwhile, the same to tty_register_device
> > and tty_unregister_device.
>
> Again, this is not an explanation. Why do think it is needed? What could
> possibly go wrong if you don't change the code like you propose?
>
> > 2. From the function naming style,
> >
> > tty_port_register_device - tty_port_unregister_device;
> > tty_register_device - tty_unregister_device
>
> Yes, the naming suggests you should be using the port helper and it is
> ok to do so, but again, it is not a requirement (unless you're using the
> serdev variant).
OK, I see. Thanks for your information.
Next time I will double-check the underlying details of each function
to verify if they are in pairs.
>
> > > > Fixes: a6afd9f3e819 ("tty: move a number of tty drivers from drivers/char/ to drivers/tty/")
> > >
> > > Please try a little harder, that's clearly not the commit that changed
> > > to the port registration helper.
> > >
> > > > Cc: stable@vger.kernel.org
> > >
> > > Why do you think this is stable material? (hint: it is not)
> >
> > From the documentation, this label could make the patch automatically
> > go to stable tree. And stable tree is also using the incorrect api.
>
> No, it is not using an "incorrect api". There is nothing wrong with
> current code. And it certainly does not need to be changed in stable.
>
> Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-21 13:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 11:33 [PATCH v2] tty: nozomi: tty_unregister_device -> tty_port_unregister_device Dongliang Mu
2021-07-21 12:00 ` Johan Hovold
2021-07-21 12:53 ` Dongliang Mu
2021-07-21 13:09 ` Johan Hovold
2021-07-21 13:35 ` Dongliang Mu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).