[PATCH v1 00/13] serial: 8250_exar: Clean up the driver

Andy Shevchenko posted 13 patches 1 year, 7 months ago
There is a newer version of this series
drivers/tty/serial/8250/8250_exar.c | 454 ++++++++++++----------------
1 file changed, 200 insertions(+), 254 deletions(-)
[PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Andy Shevchenko 1 year, 7 months ago
After a rework for CONNTECH was done, the driver may need a bit of
love in order to become less verbose (in terms of indentation and
code duplication) and hence easier to read.

This clean up series fixes a couple of (not so critical) issues and
cleans up the recently added code. No functional change indented by
the cleaning up part.

Andy Shevchenko (13):
  serial: 8250_exar: Don't return positive values as error codes
  serial: 8250_exar: Describe all parameters in kernel doc
  serial: 8250_exar: Kill CTI_PCI_DEVICE()
  serial: 8250_exar: Use PCI_SUBVENDOR_ID_IBM for subvendor ID
  serial: 8250_exar: Trivia typo fixes
  serial: 8250_exar: Extract cti_board_init_osc_freq() helper
  serial: 8250_exar: Kill unneeded ->board_init()
  serial: 8250_exar: Decrease indentation level
  serial: 8250_exar: Return directly from switch-cases
  serial: 8250_exar: Switch to use dev_err_probe()
  serial: 8250_exar: Use BIT() in exar_ee_read()
  serial: 8250_exar: Make type of bit the same in exar_ee_*_bit()
  serial: 8250_exar: Keep the includes sorted

 drivers/tty/serial/8250/8250_exar.c | 454 ++++++++++++----------------
 1 file changed, 200 insertions(+), 254 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Parker Newman 1 year, 7 months ago
On Thu,  2 May 2024 17:43:54 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> After a rework for CONNTECH was done, the driver may need a bit of
> love in order to become less verbose (in terms of indentation and
> code duplication) and hence easier to read.
>
> This clean up series fixes a couple of (not so critical) issues and
> cleans up the recently added code. No functional change indented by
> the cleaning up part.
>

Just an FYI I submitted a patch series that fixed several of these issues but I
think it fell through the cracks (I know everyone is very busy!).

Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/

I believe my previous patch series is no longer required. This one fixes
everything.

Thanks,
Parker

> Andy Shevchenko (13):
>   serial: 8250_exar: Don't return positive values as error codes
>   serial: 8250_exar: Describe all parameters in kernel doc
>   serial: 8250_exar: Kill CTI_PCI_DEVICE()
>   serial: 8250_exar: Use PCI_SUBVENDOR_ID_IBM for subvendor ID
>   serial: 8250_exar: Trivia typo fixes
>   serial: 8250_exar: Extract cti_board_init_osc_freq() helper
>   serial: 8250_exar: Kill unneeded ->board_init()
>   serial: 8250_exar: Decrease indentation level
>   serial: 8250_exar: Return directly from switch-cases
>   serial: 8250_exar: Switch to use dev_err_probe()
>   serial: 8250_exar: Use BIT() in exar_ee_read()
>   serial: 8250_exar: Make type of bit the same in exar_ee_*_bit()
>   serial: 8250_exar: Keep the includes sorted
>
>  drivers/tty/serial/8250/8250_exar.c | 454 ++++++++++++----------------
>  1 file changed, 200 insertions(+), 254 deletions(-)
>
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> On Thu,  2 May 2024 17:43:54 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > After a rework for CONNTECH was done, the driver may need a bit of
> > love in order to become less verbose (in terms of indentation and
> > code duplication) and hence easier to read.
> >
> > This clean up series fixes a couple of (not so critical) issues and
> > cleans up the recently added code. No functional change indented by
> > the cleaning up part.
> >
> 
> Just an FYI I submitted a patch series that fixed several of these issues but I
> think it fell through the cracks (I know everyone is very busy!).
> 
> Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> 
> I believe my previous patch series is no longer required. This one fixes
> everything.

I haven't noticed that, if it contains duplicated patches, I may replace mine
with yours if you insist.

In any case it's better to reply there that you prefer this series to be
applied, so Greg will not pick it up.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Parker Newman 1 year, 7 months ago
On Thu, 2 May 2024 19:01:01 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > On Thu,  2 May 2024 17:43:54 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > After a rework for CONNTECH was done, the driver may need a bit of
> > > love in order to become less verbose (in terms of indentation and
> > > code duplication) and hence easier to read.
> > >
> > > This clean up series fixes a couple of (not so critical) issues and
> > > cleans up the recently added code. No functional change indented by
> > > the cleaning up part.
> > >
> >
> > Just an FYI I submitted a patch series that fixed several of these issues but I
> > think it fell through the cracks (I know everyone is very busy!).
> >
> > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> >
> > I believe my previous patch series is no longer required. This one fixes
> > everything.
>
> I haven't noticed that, if it contains duplicated patches, I may replace mine
> with yours if you insist.
>
> In any case it's better to reply there that you prefer this series to be
> applied, so Greg will not pick it up.
>

I do not have a preference. I am fine with using yours if it is easier on
the maintainers.

I will send a reply on my previous series that it is not needed and link
to this. I am new to the mailing lists so I didn't know what the procedure
is for this situation.

Thanks for the fixes :),
Parker
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> On Thu, 2 May 2024 19:01:01 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > On Thu,  2 May 2024 17:43:54 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > love in order to become less verbose (in terms of indentation and
> > > > code duplication) and hence easier to read.
> > > >
> > > > This clean up series fixes a couple of (not so critical) issues and
> > > > cleans up the recently added code. No functional change indented by
> > > > the cleaning up part.
> > >
> > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > think it fell through the cracks (I know everyone is very busy!).
> > >
> > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > >
> > > I believe my previous patch series is no longer required. This one fixes
> > > everything.
> >
> > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > with yours if you insist.
> >
> > In any case it's better to reply there that you prefer this series to be
> > applied, so Greg will not pick it up.
> >
> 
> I do not have a preference. I am fine with using yours if it is easier on
> the maintainers.

Up to you, there is no issue to take your patches in case they are the same
(or quite similar) as mine. I can pick them up, just tell me if you want this
to happen with a list of the patches (as mail Message-Id).

> I will send a reply on my previous series that it is not needed and link
> to this. I am new to the mailing lists so I didn't know what the procedure
> is for this situation.

It's not about mailing lists, it's just a common sense.

> Thanks for the fixes :),

You are welcome!

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Parker Newman 1 year, 7 months ago
On Thu, 2 May 2024 20:22:47 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> > On Thu, 2 May 2024 19:01:01 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > > On Thu,  2 May 2024 17:43:54 +0300
> > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > > love in order to become less verbose (in terms of indentation and
> > > > > code duplication) and hence easier to read.
> > > > >
> > > > > This clean up series fixes a couple of (not so critical) issues and
> > > > > cleans up the recently added code. No functional change indented by
> > > > > the cleaning up part.
> > > >
> > > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > > think it fell through the cracks (I know everyone is very busy!).
> > > >
> > > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > > >
> > > > I believe my previous patch series is no longer required. This one fixes
> > > > everything.
> > >
> > > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > > with yours if you insist.
> > >
> > > In any case it's better to reply there that you prefer this series to be
> > > applied, so Greg will not pick it up.
> > >
> >
> > I do not have a preference. I am fine with using yours if it is easier on
> > the maintainers.
>
> Up to you, there is no issue to take your patches in case they are the same
> (or quite similar) as mine. I can pick them up, just tell me if you want this
> to happen with a list of the patches (as mail Message-Id).
>

Just use yours.
Thanks,
Parker

> > I will send a reply on my previous series that it is not needed and link
> > to this. I am new to the mailing lists so I didn't know what the procedure
> > is for this situation.
>
> It's not about mailing lists, it's just a common sense.
>
> > Thanks for the fixes :),
>
> You are welcome!
>
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 01:49:49PM -0400, Parker Newman wrote:
> On Thu, 2 May 2024 20:22:47 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> > > On Thu, 2 May 2024 19:01:01 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > > > On Thu,  2 May 2024 17:43:54 +0300
> > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > > > love in order to become less verbose (in terms of indentation and
> > > > > > code duplication) and hence easier to read.
> > > > > >
> > > > > > This clean up series fixes a couple of (not so critical) issues and
> > > > > > cleans up the recently added code. No functional change indented by
> > > > > > the cleaning up part.
> > > > >
> > > > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > > > think it fell through the cracks (I know everyone is very busy!).
> > > > >
> > > > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > > > >
> > > > > I believe my previous patch series is no longer required. This one fixes
> > > > > everything.
> > > >
> > > > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > > > with yours if you insist.
> > > >
> > > > In any case it's better to reply there that you prefer this series to be
> > > > applied, so Greg will not pick it up.
> > > >
> > >
> > > I do not have a preference. I am fine with using yours if it is easier on
> > > the maintainers.
> >
> > Up to you, there is no issue to take your patches in case they are the same
> > (or quite similar) as mine. I can pick them up, just tell me if you want this
> > to happen with a list of the patches (as mail Message-Id).
> 
> Just use yours.

Okay, thanks!

If you are going to test, better to pay attention to the BIT() conversion patch
as Ilpo noted an issue. I believe it's easy to drop (via local git-rebase run)
or move and test separately.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Parker Newman 1 year, 7 months ago
On Thu, 2 May 2024 21:01:54 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, May 02, 2024 at 01:49:49PM -0400, Parker Newman wrote:
> > On Thu, 2 May 2024 20:22:47 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> > > > On Thu, 2 May 2024 19:01:01 +0300
> > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > > > > On Thu,  2 May 2024 17:43:54 +0300
> > > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > >
> > > > > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > > > > love in order to become less verbose (in terms of indentation and
> > > > > > > code duplication) and hence easier to read.
> > > > > > >
> > > > > > > This clean up series fixes a couple of (not so critical) issues and
> > > > > > > cleans up the recently added code. No functional change indented by
> > > > > > > the cleaning up part.
> > > > > >
> > > > > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > > > > think it fell through the cracks (I know everyone is very busy!).
> > > > > >
> > > > > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > > > > >
> > > > > > I believe my previous patch series is no longer required. This one fixes
> > > > > > everything.
> > > > >
> > > > > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > > > > with yours if you insist.
> > > > >
> > > > > In any case it's better to reply there that you prefer this series to be
> > > > > applied, so Greg will not pick it up.
> > > > >
> > > >
> > > > I do not have a preference. I am fine with using yours if it is easier on
> > > > the maintainers.
> > >
> > > Up to you, there is no issue to take your patches in case they are the same
> > > (or quite similar) as mine. I can pick them up, just tell me if you want this
> > > to happen with a list of the patches (as mail Message-Id).
> >
> > Just use yours.
>
> Okay, thanks!
>
> If you are going to test, better to pay attention to the BIT() conversion patch
> as Ilpo noted an issue. I believe it's easy to drop (via local git-rebase run)
> or move and test separately.
>

I am working on testing now but patches 7 and 12 did not apply with git am.
Both failed around lines 1095/1096.
I can apply them manually but I may be using the wrong branch (tty-next).
Which branch/commit did you create your patches from? I don't see it in the
patch submission.
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Parker Newman 1 year, 7 months ago
On Fri, 3 May 2024 08:36:38 -0400
Parker Newman <parker@finest.io> wrote:

> On Thu, 2 May 2024 21:01:54 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > On Thu, May 02, 2024 at 01:49:49PM -0400, Parker Newman wrote:
> > > On Thu, 2 May 2024 20:22:47 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> > > > > On Thu, 2 May 2024 19:01:01 +0300
> > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > > > > > On Thu,  2 May 2024 17:43:54 +0300
> > > > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > >
> > > > > > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > > > > > love in order to become less verbose (in terms of indentation and
> > > > > > > > code duplication) and hence easier to read.
> > > > > > > >
> > > > > > > > This clean up series fixes a couple of (not so critical) issues and
> > > > > > > > cleans up the recently added code. No functional change indented by
> > > > > > > > the cleaning up part.
> > > > > > >
> > > > > > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > > > > > think it fell through the cracks (I know everyone is very busy!).
> > > > > > >
> > > > > > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > > > > > >
> > > > > > > I believe my previous patch series is no longer required. This one fixes
> > > > > > > everything.
> > > > > >
> > > > > > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > > > > > with yours if you insist.
> > > > > >
> > > > > > In any case it's better to reply there that you prefer this series to be
> > > > > > applied, so Greg will not pick it up.
> > > > > >
> > > > >
> > > > > I do not have a preference. I am fine with using yours if it is easier on
> > > > > the maintainers.
> > > >
> > > > Up to you, there is no issue to take your patches in case they are the same
> > > > (or quite similar) as mine. I can pick them up, just tell me if you want this
> > > > to happen with a list of the patches (as mail Message-Id).
> > >
> > > Just use yours.
> >
> > Okay, thanks!
> >
> > If you are going to test, better to pay attention to the BIT() conversion patch
> > as Ilpo noted an issue. I believe it's easy to drop (via local git-rebase run)
> > or move and test separately.
> >
>
> I am working on testing now but patches 7 and 12 did not apply with git am.
> Both failed around lines 1095/1096.
> I can apply them manually but I may be using the wrong branch (tty-next).
> Which branch/commit did you create your patches from? I don't see it in the
> patch submission.

I figured it out. git am was applying the typo fix patch out of order.
Sorry, I didn't notice that. Patches should be fine.

I can do a final test once you decide what to do with the BIT() conversion patch.

Parker
Re: [PATCH v1 00/13] serial: 8250_exar: Clean up the driver
Posted by Andy Shevchenko 1 year, 7 months ago
On Fri, May 03, 2024 at 10:47:30AM -0400, Parker Newman wrote:
> On Fri, 3 May 2024 08:36:38 -0400
> Parker Newman <parker@finest.io> wrote:
> 
> > On Thu, 2 May 2024 21:01:54 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > On Thu, May 02, 2024 at 01:49:49PM -0400, Parker Newman wrote:
> > > > On Thu, 2 May 2024 20:22:47 +0300
> > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Thu, May 02, 2024 at 12:08:40PM -0400, Parker Newman wrote:
> > > > > > On Thu, 2 May 2024 19:01:01 +0300
> > > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Thu, May 02, 2024 at 11:46:45AM -0400, Parker Newman wrote:
> > > > > > > > On Thu,  2 May 2024 17:43:54 +0300
> > > > > > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > >
> > > > > > > > > After a rework for CONNTECH was done, the driver may need a bit of
> > > > > > > > > love in order to become less verbose (in terms of indentation and
> > > > > > > > > code duplication) and hence easier to read.
> > > > > > > > >
> > > > > > > > > This clean up series fixes a couple of (not so critical) issues and
> > > > > > > > > cleans up the recently added code. No functional change indented by
> > > > > > > > > the cleaning up part.
> > > > > > > >
> > > > > > > > Just an FYI I submitted a patch series that fixed several of these issues but I
> > > > > > > > think it fell through the cracks (I know everyone is very busy!).
> > > > > > > >
> > > > > > > > Link: https://lore.kernel.org/linux-serial/cover.1713533298.git.pnewman@connecttech.com/
> > > > > > > >
> > > > > > > > I believe my previous patch series is no longer required. This one fixes
> > > > > > > > everything.
> > > > > > >
> > > > > > > I haven't noticed that, if it contains duplicated patches, I may replace mine
> > > > > > > with yours if you insist.
> > > > > > >
> > > > > > > In any case it's better to reply there that you prefer this series to be
> > > > > > > applied, so Greg will not pick it up.
> > > > > > >
> > > > > >
> > > > > > I do not have a preference. I am fine with using yours if it is easier on
> > > > > > the maintainers.
> > > > >
> > > > > Up to you, there is no issue to take your patches in case they are the same
> > > > > (or quite similar) as mine. I can pick them up, just tell me if you want this
> > > > > to happen with a list of the patches (as mail Message-Id).
> > > >
> > > > Just use yours.
> > >
> > > Okay, thanks!
> > >
> > > If you are going to test, better to pay attention to the BIT() conversion patch
> > > as Ilpo noted an issue. I believe it's easy to drop (via local git-rebase run)
> > > or move and test separately.
> > >
> >
> > I am working on testing now but patches 7 and 12 did not apply with git am.
> > Both failed around lines 1095/1096.
> > I can apply them manually but I may be using the wrong branch (tty-next).
> > Which branch/commit did you create your patches from? I don't see it in the
> > patch submission.
> 
> I figured it out. git am was applying the typo fix patch out of order.
> Sorry, I didn't notice that. Patches should be fine.
> 
> I can do a final test once you decide what to do with the BIT() conversion patch.

Can you revert it and test the rest? So we will know that they are okay.
Or does the above implies that you already performed such a test?

-- 
With Best Regards,
Andy Shevchenko