Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH] kgdboc: Fix restrict error
From: Laura Abbott @ 2018-09-10 23:20 UTC (permalink / raw)
  To: Jason Wessel, Daniel Thompson
  Cc: Laura Abbott, Greg Kroah-Hartman, Jiri Slaby, kgdb-bugreport,
	linux-serial, linux-kernel

There's an error when compiled with restrict:

drivers/tty/serial/kgdboc.c: In function ‘configure_kgdboc’:
drivers/tty/serial/kgdboc.c:137:2: error: ‘strcpy’ source argument is the same
as destination [-Werror=restrict]
  strcpy(config, opt);
  ^~~~~~~~~~~~~~~~~~~

As the error implies, this is from trying to use config as both source and
destination. Drop the call to the function where config is the argument
since nothing else happens in the function.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/tty/serial/kgdboc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index b4ba2b1dab76..dd503743bbff 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -154,15 +154,13 @@ static int configure_kgdboc(void)
 {
 	struct tty_driver *p;
 	int tty_line = 0;
-	int err;
+	int err = -ENODEV;
 	char *cptr = config;
 	struct console *cons;
 
-	err = kgdboc_option_setup(config);
-	if (err || !strlen(config) || isspace(config[0]))
+	if (!strlen(config) || isspace(config[0]))
 		goto noconfig;
 
-	err = -ENODEV;
 	kgdboc_io_ops.is_console = 0;
 	kgdb_tty_driver = NULL;
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 1/5] serial: sprd: Remove unused structure
From: Baolin Wang @ 2018-09-11  6:09 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: Greg Kroah-Hartman, jslaby, Orson Zhai, Mark Brown, linux-serial,
	Linux Kernel Mailing List
In-Reply-To: <CAAfSe-tVJ1P30y+aj-2AsEtsbQ=O3vADxZNc_UW6ri92suGMgA@mail.gmail.com>

Hi,

On 4 September 2018 at 14:52, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> On 11 August 2018 at 09:34, Baolin Wang <baolin.wang@linaro.org> wrote:
>> Remove the unused reg_backup structure.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>
> Acked-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>

Thanks Chunyan.

Greg, could you apply this patch set if there are no other issues? Thanks.

>> ---
>>  drivers/tty/serial/sprd_serial.c | 11 -----------
>>  1 file changed, 11 deletions(-)
>>
>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
>> index 828f114..1b0e3fb 100644
>> --- a/drivers/tty/serial/sprd_serial.c
>> +++ b/drivers/tty/serial/sprd_serial.c
>> @@ -95,19 +95,8 @@
>>  #define SPRD_IMSR_BREAK_DETECT         BIT(7)
>>  #define SPRD_IMSR_TIMEOUT              BIT(13)
>>
>> -struct reg_backup {
>> -       u32 ien;
>> -       u32 ctrl0;
>> -       u32 ctrl1;
>> -       u32 ctrl2;
>> -       u32 clkd0;
>> -       u32 clkd1;
>> -       u32 dspwait;
>> -};
>> -
>>  struct sprd_uart_port {
>>         struct uart_port port;
>> -       struct reg_backup reg_bak;
>>         char name[16];
>>  };
>>
>> --
>> 1.9.1
>>



-- 
Baolin Wang
Best Regards

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Radu Nicolae Pirea @ 2018-09-11  8:14 UTC (permalink / raw)
  To: Lee Jones, Radu Pirea
  Cc: broonie, nicolas.ferre, alexandre.belloni, richard.genoud,
	robh+dt, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
	linux-kernel, devicetree, linux-serial
In-Reply-To: <20180910151803.GX28860@dell>

Hi Lee,

Thank you for taking the patch series, but you didn't take the latest
version(v12) :)


On Mon, 2018-09-10 at 16:18 +0100, Lee Jones wrote:
> Enjoy!
> 
> The following changes since commit
> 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> 
>   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-
> mfd-spi-tty-v4.20
> 
> for you to fetch changes up to
> c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> 
>   tty/serial: atmel: Change the driver to work under at91-usart MFD
> (2018-09-10 16:12:43 +0100)
> 
> ----------------------------------------------------------------
> Immutable branch between MFD, SPI and TTY due for the v4.20 merge
> window
> 
> ----------------------------------------------------------------
> Radu Pirea (6):
>       MAINTAINERS: Add AT91 USART MFD entry
>       dt-bindings: Add binding for atmel-usart in SPI mode
>       mfd: at91-usart: Add MFD driver for USART
>       MAINTAINERS: Add AT91 USART SPI entry
>       spi: at91-usart: Add driver for at91-usart as SPI
>       tty/serial: atmel: Change the driver to work under at91-usart
> MFD
> 
>  .../bindings/{serial => mfd}/atmel-usart.txt       |  25 +-
>  MAINTAINERS                                        |  16 +
>  drivers/mfd/Kconfig                                |   9 +
>  drivers/mfd/Makefile                               |   1 +
>  drivers/mfd/at91-usart.c                           |  71 ++++
>  drivers/spi/Kconfig                                |   8 +
>  drivers/spi/Makefile                               |   1 +
>  drivers/spi/spi-at91-usart.c                       | 432
> +++++++++++++++++++++
>  drivers/tty/serial/Kconfig                         |   1 +
>  drivers/tty/serial/atmel_serial.c                  |  42 +-
>  include/dt-bindings/mfd/at91-usart.h               |  17 +
>  11 files changed, 606 insertions(+), 17 deletions(-)
>  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-
> usart.txt (76%)
>  create mode 100644 drivers/mfd/at91-usart.c
>  create mode 100644 drivers/spi/spi-at91-usart.c
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11  8:45 UTC (permalink / raw)
  To: Radu Pirea
  Cc: broonie, nicolas.ferre, alexandre.belloni, richard.genoud,
	robh+dt, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
	linux-kernel, devicetree, linux-serial
In-Reply-To: <20180910151803.GX28860@dell>

On Mon, 10 Sep 2018, Lee Jones wrote:

> Enjoy!
> 
> The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> 
>   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
> 
> for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> 
>   tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
> 
> ----------------------------------------------------------------
> Immutable branch between MFD, SPI and TTY due for the v4.20 merge window

Due to a comedy of errors, I'm voiding this pull-request.

Please stand-by for a follow-up.

> Radu Pirea (6):
>       MAINTAINERS: Add AT91 USART MFD entry
>       dt-bindings: Add binding for atmel-usart in SPI mode
>       mfd: at91-usart: Add MFD driver for USART
>       MAINTAINERS: Add AT91 USART SPI entry
>       spi: at91-usart: Add driver for at91-usart as SPI
>       tty/serial: atmel: Change the driver to work under at91-usart MFD
> 
>  .../bindings/{serial => mfd}/atmel-usart.txt       |  25 +-
>  MAINTAINERS                                        |  16 +
>  drivers/mfd/Kconfig                                |   9 +
>  drivers/mfd/Makefile                               |   1 +
>  drivers/mfd/at91-usart.c                           |  71 ++++
>  drivers/spi/Kconfig                                |   8 +
>  drivers/spi/Makefile                               |   1 +
>  drivers/spi/spi-at91-usart.c                       | 432 +++++++++++++++++++++
>  drivers/tty/serial/Kconfig                         |   1 +
>  drivers/tty/serial/atmel_serial.c                  |  42 +-
>  include/dt-bindings/mfd/at91-usart.h               |  17 +
>  11 files changed, 606 insertions(+), 17 deletions(-)
>  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
>  create mode 100644 drivers/mfd/at91-usart.c
>  create mode 100644 drivers/spi/spi-at91-usart.c
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11  8:51 UTC (permalink / raw)
  To: Radu Nicolae Pirea
  Cc: Radu Pirea, broonie, nicolas.ferre, alexandre.belloni,
	richard.genoud, robh+dt, mark.rutland, gregkh, linux-spi,
	linux-arm-kernel, linux-kernel, devicetree, linux-serial
In-Reply-To: <bd9104b702feec556ed01311e9efcb96790131d0.camel@upb.ro>

On Tue, 11 Sep 2018, Radu Nicolae Pirea wrote:

> Hi Lee,
> 
> Thank you for taking the patch series, but you didn't take the latest
> version(v12) :)

Both were marked for merging in my inbox, and since I am working
though my (300+) post-vacation emails chronologically v11 was handled
first.

It appears as though there is an issue in the way 'struct
of_device_id' was declared too.  The header file in which it was
declared used to be included by proxy, however a separation has
recently been made causing build errors.

I will fix both issues and re-submit the PR.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Greg KH @ 2018-09-11  9:01 UTC (permalink / raw)
  To: Lee Jones
  Cc: Radu Pirea, broonie, nicolas.ferre, alexandre.belloni,
	richard.genoud, robh+dt, mark.rutland, linux-spi,
	linux-arm-kernel, linux-kernel, devicetree, linux-serial
In-Reply-To: <20180911084548.GC4185@dell>

On Tue, Sep 11, 2018 at 09:45:48AM +0100, Lee Jones wrote:
> On Mon, 10 Sep 2018, Lee Jones wrote:
> 
> > Enjoy!
> > 
> > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > 
> >   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > 
> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
> > 
> > for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> > 
> >   tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
> > 
> > ----------------------------------------------------------------
> > Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
> 
> Due to a comedy of errors, I'm voiding this pull-request.

Um, I can't do that as my tree can not be rebased :(

I can take follow-on patches, so I will go apply the patch posted so
that my tree builds again.  I recommend you just apply it as well.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Lee Jones @ 2018-09-11  9:33 UTC (permalink / raw)
  To: Radu Pirea
  Cc: robh+dt, mark.rutland, nicolas.ferre, alexandre.belloni, gregkh,
	broonie, jslaby, richard.genoud, davem, mchehab+samsung, akpm,
	arnd, devicetree, linux-arm-kernel, linux-kernel, linux-serial,
	linux-spi
In-Reply-To: <20180904111310.4049-1-radu_nicolae.pirea@upb.ro>

On Tue, 04 Sep 2018, Radu Pirea wrote:
> Radu Pirea (6):
>   MAINTAINERS: add at91 usart mfd driver
>   dt-bindings: add binding for atmel-usart in SPI mode
>   mfd: at91-usart: added mfd driver for usart
>   MAINTAINERS: add at91 usart spi driver
>   spi: at91-usart: add driver for at91-usart as spi
>   tty/serial: atmel: change the driver to work under at91-usart mfd
> 
>  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
>  MAINTAINERS                                   |  16 +
>  drivers/mfd/Kconfig                           |   9 +
>  drivers/mfd/Makefile                          |   1 +
>  drivers/mfd/at91-usart.c                      |  71 +++
>  drivers/spi/Kconfig                           |   8 +
>  drivers/spi/Makefile                          |   1 +
>  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
>  drivers/tty/serial/Kconfig                    |   1 +
>  drivers/tty/serial/atmel_serial.c             |  42 +-
>  include/dt-bindings/mfd/at91-usart.h          |  17 +
>  11 files changed, 606 insertions(+), 17 deletions(-)
>  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
>  create mode 100644 drivers/mfd/at91-usart.c
>  create mode 100644 drivers/spi/spi-at91-usart.c
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h

Seeing as this patch-set has caused some issues this morning, I took
the liberty to peruse back into its history to figure out where things
started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
I did!

My Acked-by has been attached to the MFD portion since v5, which is
why the code hasn't caught my eye before today.  I reviewed the
relocation of the *binding document* (serial => mfd with no changes)
in v4 and nothing else.  It appears as though you mistakenly added it
to the *MFD driver* instead.  This explains my confusion in v10 when I
told you I'd already reviewed the binding document.

As I said, I have re-reviewed the MFD driver and I'm afraid to say
that I do not like what I see.  Besides the missing header file and
the whitespace tabbing errors, I do not agree with the implementation.
Using MFD as a shim to hack around driver selection is not a valid
use-case.

What's stopping you from just using the compatible string directly to
select which driver you need to probe?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Alexandre Belloni @ 2018-09-11  9:39 UTC (permalink / raw)
  To: Lee Jones
  Cc: Radu Pirea, robh+dt, mark.rutland, nicolas.ferre, gregkh, broonie,
	jslaby, richard.genoud, davem, mchehab+samsung, akpm, arnd,
	devicetree, linux-arm-kernel, linux-kernel, linux-serial,
	linux-spi
In-Reply-To: <20180911093356.GE4185@dell>

On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> On Tue, 04 Sep 2018, Radu Pirea wrote:
> > Radu Pirea (6):
> >   MAINTAINERS: add at91 usart mfd driver
> >   dt-bindings: add binding for atmel-usart in SPI mode
> >   mfd: at91-usart: added mfd driver for usart
> >   MAINTAINERS: add at91 usart spi driver
> >   spi: at91-usart: add driver for at91-usart as spi
> >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > 
> >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> >  MAINTAINERS                                   |  16 +
> >  drivers/mfd/Kconfig                           |   9 +
> >  drivers/mfd/Makefile                          |   1 +
> >  drivers/mfd/at91-usart.c                      |  71 +++
> >  drivers/spi/Kconfig                           |   8 +
> >  drivers/spi/Makefile                          |   1 +
> >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> >  drivers/tty/serial/Kconfig                    |   1 +
> >  drivers/tty/serial/atmel_serial.c             |  42 +-
> >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> >  11 files changed, 606 insertions(+), 17 deletions(-)
> >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> >  create mode 100644 drivers/mfd/at91-usart.c
> >  create mode 100644 drivers/spi/spi-at91-usart.c
> >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 
> Seeing as this patch-set has caused some issues this morning, I took
> the liberty to peruse back into its history to figure out where things
> started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> I did!
> 
> My Acked-by has been attached to the MFD portion since v5, which is
> why the code hasn't caught my eye before today.  I reviewed the
> relocation of the *binding document* (serial => mfd with no changes)
> in v4 and nothing else.  It appears as though you mistakenly added it
> to the *MFD driver* instead.  This explains my confusion in v10 when I
> told you I'd already reviewed the binding document.
> 
> As I said, I have re-reviewed the MFD driver and I'm afraid to say
> that I do not like what I see.  Besides the missing header file and
> the whitespace tabbing errors, I do not agree with the implementation.
> Using MFD as a shim to hack around driver selection is not a valid
> use-case.
> 
> What's stopping you from just using the compatible string directly to
> select which driver you need to probe?
> 

Then you'd have multiple compatible strings for the same IP which is a
big no-no.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11  9:44 UTC (permalink / raw)
  To: Greg KH
  Cc: Radu Pirea, broonie, nicolas.ferre, alexandre.belloni,
	richard.genoud, robh+dt, mark.rutland, linux-spi,
	linux-arm-kernel, linux-kernel, devicetree, linux-serial
In-Reply-To: <20180911090132.GB25161@kroah.com>

On Tue, 11 Sep 2018, Greg KH wrote:

> On Tue, Sep 11, 2018 at 09:45:48AM +0100, Lee Jones wrote:
> > On Mon, 10 Sep 2018, Lee Jones wrote:
> > 
> > > Enjoy!
> > > 
> > > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > > 
> > >   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
> > > 
> > > for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> > > 
> > >   tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
> > > 
> > > ----------------------------------------------------------------
> > > Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
> > 
> > Due to a comedy of errors, I'm voiding this pull-request.
> 
> Um, I can't do that as my tree can not be rebased :(

Oh dear. :(

> I can take follow-on patches, so I will go apply the patch posted so
> that my tree builds again.  I recommend you just apply it as well.

What a pickle.  As per my last email, it looks as though this set was
applied under false pretences.  The MFD patch which was carrying my
Ack was doing so incorrectly.  The author mistakenly applied it to the
wrong patch.  After re-review (actually initial review) I wish to NACK
the implementation.

My suggestion would be to sit tight, rather than do anything rash and
see how this plays out.  After which we can fix your tree.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Nicolas Ferre @ 2018-09-11  9:56 UTC (permalink / raw)
  To: Lee Jones, Greg KH, Radu Pirea
  Cc: broonie, alexandre.belloni, richard.genoud, robh+dt, mark.rutland,
	linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial
In-Reply-To: <20180911094430.GG4185@dell>

On 11/09/2018 at 11:44, Lee Jones wrote:
> On Tue, 11 Sep 2018, Greg KH wrote:
> 
>> On Tue, Sep 11, 2018 at 09:45:48AM +0100, Lee Jones wrote:
>>> On Mon, 10 Sep 2018, Lee Jones wrote:
>>>
>>>> Enjoy!
>>>>
>>>> The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
>>>>
>>>>    Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>    git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
>>>>
>>>> for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
>>>>
>>>>    tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
>>>>
>>>> ----------------------------------------------------------------
>>>> Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
>>>
>>> Due to a comedy of errors, I'm voiding this pull-request.
>>
>> Um, I can't do that as my tree can not be rebased :(
> 
> Oh dear. :(
> 
>> I can take follow-on patches, so I will go apply the patch posted so
>> that my tree builds again.  I recommend you just apply it as well.
> 
> What a pickle.  As per my last email, it looks as though this set was
> applied under false pretences.  The MFD patch which was carrying my
> Ack was doing so incorrectly.  The author mistakenly applied it to the
> wrong patch.  After re-review (actually initial review) I wish to NACK
> the implementation.

What? We discussed this to the bone!

Radu followed all the recommendations, collected all the feedback from 
major subsystems SPI and USART, resent the series again.

It was then forgotten. Wrong series was finally applied (which could 
trigger 0-day reports, I warn you)... and now we are discussing about 
the implementation of *v2* (we're at v12)!

> My suggestion would be to sit tight, rather than do anything rash and
> see how this plays out.  After which we can fix your tree.

Come on, Greg's tree is fixed now. We can catch-up with v11..v12 changes 
with patches and cleanup the mess together.

Just one misalignment with one patch merged for v4.19-rc1, fixed the 
same day, cannot generate such a reaction after months of development.

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Greg KH @ 2018-09-11 10:03 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Lee Jones, Radu Pirea, broonie, alexandre.belloni, richard.genoud,
	robh+dt, mark.rutland, linux-spi, linux-arm-kernel, linux-kernel,
	devicetree, linux-serial
In-Reply-To: <743b5545-e942-b6f4-4ce6-8e10950a05fc@microchip.com>

On Tue, Sep 11, 2018 at 11:56:49AM +0200, Nicolas Ferre wrote:
> On 11/09/2018 at 11:44, Lee Jones wrote:
> > On Tue, 11 Sep 2018, Greg KH wrote:
> > 
> > > On Tue, Sep 11, 2018 at 09:45:48AM +0100, Lee Jones wrote:
> > > > On Mon, 10 Sep 2018, Lee Jones wrote:
> > > > 
> > > > > Enjoy!
> > > > > 
> > > > > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > > > > 
> > > > >    Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > > > > 
> > > > > are available in the Git repository at:
> > > > > 
> > > > >    git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
> > > > > 
> > > > > for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> > > > > 
> > > > >    tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > > Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
> > > > 
> > > > Due to a comedy of errors, I'm voiding this pull-request.
> > > 
> > > Um, I can't do that as my tree can not be rebased :(
> > 
> > Oh dear. :(
> > 
> > > I can take follow-on patches, so I will go apply the patch posted so
> > > that my tree builds again.  I recommend you just apply it as well.
> > 
> > What a pickle.  As per my last email, it looks as though this set was
> > applied under false pretences.  The MFD patch which was carrying my
> > Ack was doing so incorrectly.  The author mistakenly applied it to the
> > wrong patch.  After re-review (actually initial review) I wish to NACK
> > the implementation.
> 
> What? We discussed this to the bone!
> 
> Radu followed all the recommendations, collected all the feedback from major
> subsystems SPI and USART, resent the series again.
> 
> It was then forgotten. Wrong series was finally applied (which could trigger
> 0-day reports, I warn you)... and now we are discussing about the
> implementation of *v2* (we're at v12)!
> 
> > My suggestion would be to sit tight, rather than do anything rash and
> > see how this plays out.  After which we can fix your tree.
> 
> Come on, Greg's tree is fixed now. We can catch-up with v11..v12 changes
> with patches and cleanup the mess together.
> 
> Just one misalignment with one patch merged for v4.19-rc1, fixed the same
> day, cannot generate such a reaction after months of development.

I agree, my tree is now fixed, no harm done.  Just apply the same patch
to whomever also pulled it into their trees and all is good.

thanks,

greg k-h

^ permalink raw reply

* Re: [GIT PULL] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11 10:13 UTC (permalink / raw)
  To: Greg KH
  Cc: Nicolas Ferre, Radu Pirea, broonie, alexandre.belloni,
	richard.genoud, robh+dt, mark.rutland, linux-spi,
	linux-arm-kernel, linux-kernel, devicetree, linux-serial
In-Reply-To: <20180911100336.GA3822@kroah.com>

On Tue, 11 Sep 2018, Greg KH wrote:

> On Tue, Sep 11, 2018 at 11:56:49AM +0200, Nicolas Ferre wrote:
> > On 11/09/2018 at 11:44, Lee Jones wrote:
> > > On Tue, 11 Sep 2018, Greg KH wrote:
> > > 
> > > > On Tue, Sep 11, 2018 at 09:45:48AM +0100, Lee Jones wrote:
> > > > > On Mon, 10 Sep 2018, Lee Jones wrote:
> > > > > 
> > > > > > Enjoy!
> > > > > > 
> > > > > > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > > > > > 
> > > > > >    Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > > > > > 
> > > > > > are available in the Git repository at:
> > > > > > 
> > > > > >    git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20
> > > > > > 
> > > > > > for you to fetch changes up to c24d25317a7c6bb3053d4c193b3cf57d1e9a3e4b:
> > > > > > 
> > > > > >    tty/serial: atmel: Change the driver to work under at91-usart MFD (2018-09-10 16:12:43 +0100)
> > > > > > 
> > > > > > ----------------------------------------------------------------
> > > > > > Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
> > > > > 
> > > > > Due to a comedy of errors, I'm voiding this pull-request.
> > > > 
> > > > Um, I can't do that as my tree can not be rebased :(
> > > 
> > > Oh dear. :(
> > > 
> > > > I can take follow-on patches, so I will go apply the patch posted so
> > > > that my tree builds again.  I recommend you just apply it as well.
> > > 
> > > What a pickle.  As per my last email, it looks as though this set was
> > > applied under false pretences.  The MFD patch which was carrying my
> > > Ack was doing so incorrectly.  The author mistakenly applied it to the
> > > wrong patch.  After re-review (actually initial review) I wish to NACK
> > > the implementation.
> > 
> > What? We discussed this to the bone!
> > 
> > Radu followed all the recommendations, collected all the feedback from major
> > subsystems SPI and USART, resent the series again.
> > 
> > It was then forgotten. Wrong series was finally applied (which could trigger
> > 0-day reports, I warn you)... and now we are discussing about the
> > implementation of *v2* (we're at v12)!

This situation is frustrating.  The reason I was not part of the
discussion can be attributed to an erroneous application of my Ack
earlier in the review process.

Anyway ... since Greg has applied the pull-request I do agree that the
path of least friction would be just to apply the set. :(

> > > My suggestion would be to sit tight, rather than do anything rash and
> > > see how this plays out.  After which we can fix your tree.
> > 
> > Come on, Greg's tree is fixed now. We can catch-up with v11..v12 changes
> > with patches and cleanup the mess together.
> > 
> > Just one misalignment with one patch merged for v4.19-rc1, fixed the same
> > day, cannot generate such a reaction after months of development.
> 
> I agree, my tree is now fixed, no harm done.  Just apply the same patch
> to whomever also pulled it into their trees and all is good.

Have you applied Nicolas' patch already?  Without discussion? :((

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [GIT PULL v2] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11 10:56 UTC (permalink / raw)
  To: Radu Pirea
  Cc: broonie, nicolas.ferre, alexandre.belloni, richard.genoud,
	robh+dt, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
	linux-kernel, devicetree, linux-serial
In-Reply-To: <20180910151803.GX28860@dell>

This new pull-request contains the following changes since v1:

 - Uplift the submission version from v11 to v12
 - Include fix for build breakage due to missing Device Tree include file

Since it is based on the original pull-request, it can be pulled
in either directly on top of it or as a pull in its own right.

The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:

  Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20-1

for you to fetch changes up to 65b80dfffeabd4eb253b93d07eadde4d89c18511:

  mfd: at91-usart: Include Device Tree header (2018-09-11 11:44:56 +0100)

----------------------------------------------------------------
Immutable branch between MFD, SPI and TTY due for the v4.20 merge window (v2)

----------------------------------------------------------------
Lee Jones (4):
      dt-bindings: mfd: atmel-usart: Correct interrupts property to include IRQ number
      MAINTAINERS: Change Radu's email address
      spi: at91-usart: Make local functions static
      mfd: at91-usart: Include Device Tree header

Radu Pirea (6):
      MAINTAINERS: Add AT91 USART MFD entry
      dt-bindings: Add binding for atmel-usart in SPI mode
      mfd: at91-usart: Add MFD driver for USART
      MAINTAINERS: Add AT91 USART SPI entry
      spi: at91-usart: Add driver for at91-usart as SPI
      tty/serial: atmel: Change the driver to work under at91-usart MFD

 .../bindings/{serial => mfd}/atmel-usart.txt       |  25 +-
 MAINTAINERS                                        |  16 +
 drivers/mfd/Kconfig                                |   9 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/at91-usart.c                           |  72 ++++
 drivers/spi/Kconfig                                |   8 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-at91-usart.c                       | 432 +++++++++++++++++++++
 drivers/tty/serial/Kconfig                         |   1 +
 drivers/tty/serial/atmel_serial.c                  |  42 +-
 include/dt-bindings/mfd/at91-usart.h               |  17 +
 11 files changed, 607 insertions(+), 17 deletions(-)
 rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [GIT PULL v2] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11 11:18 UTC (permalink / raw)
  To: Radu Pirea
  Cc: robh+dt, mark.rutland, nicolas.ferre, alexandre.belloni, gregkh,
	broonie, jslaby, richard.genoud, davem, mchehab+samsung, akpm,
	arnd, devicetree, linux-arm-kernel, linux-kernel, linux-serial,
	linux-spi
In-Reply-To: <20180904111310.4049-1-radu_nicolae.pirea@upb.ro>

[In case you missed it]

This new pull-request contains the following changes since v1:

 - Uplift the submission version from v11 to v12
 - Include fix for build breakage due to missing Device Tree include file

Since it is based on the original pull-request, it can be pulled
in either directly on top of it or as a pull in its own right.

The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:

  Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20-1

for you to fetch changes up to 65b80dfffeabd4eb253b93d07eadde4d89c18511:

  mfd: at91-usart: Include Device Tree header (2018-09-11 11:44:56 +0100)

----------------------------------------------------------------
Immutable branch between MFD, SPI and TTY due for the v4.20 merge window (v2)

----------------------------------------------------------------
Lee Jones (4):
      dt-bindings: mfd: atmel-usart: Correct interrupts property to include IRQ number
      MAINTAINERS: Change Radu's email address
      spi: at91-usart: Make local functions static
      mfd: at91-usart: Include Device Tree header

Radu Pirea (6):
      MAINTAINERS: Add AT91 USART MFD entry
      dt-bindings: Add binding for atmel-usart in SPI mode
      mfd: at91-usart: Add MFD driver for USART
      MAINTAINERS: Add AT91 USART SPI entry
      spi: at91-usart: Add driver for at91-usart as SPI
      tty/serial: atmel: Change the driver to work under at91-usart MFD

 .../bindings/{serial => mfd}/atmel-usart.txt       |  25 +-
 MAINTAINERS                                        |  16 +
 drivers/mfd/Kconfig                                |   9 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/at91-usart.c                           |  72 ++++
 drivers/spi/Kconfig                                |   8 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-at91-usart.c                       | 432 +++++++++++++++++++++
 drivers/tty/serial/Kconfig                         |   1 +
 drivers/tty/serial/atmel_serial.c                  |  42 +-
 include/dt-bindings/mfd/at91-usart.h               |  17 +
 11 files changed, 607 insertions(+), 17 deletions(-)
 rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 1/3] arm64: dts: actions: s900: Enable Tx DMA for UART5
From: kbuild test robot @ 2018-09-11 12:03 UTC (permalink / raw)
  Cc: kbuild-all, vkoul, dan.j.williams, afaerber, robh+dt, gregkh,
	jslaby, linux-serial, dmaengine, liuwei, 96boards, devicetree,
	daniel.thompson, amit.kucheria, linux-arm-kernel, linux-kernel,
	hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, pn,
	edgar.righi, Manivannan Sadhasivam
In-Reply-To: <20180901164215.3683-2-manivannan.sadhasivam@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

Hi Manivannan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Manivannan-Sadhasivam/Add-slave-DMA-support-for-Actios-Semi-S900-SoC/20180903-163744
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> ERROR: Input tree has errors, aborting (use -f to force output)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61011 bytes --]

^ permalink raw reply

* Re: [PATCH] kgdboc: Fix restrict error
From: Daniel Thompson @ 2018-09-11 12:49 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Jason Wessel, Greg Kroah-Hartman, Jiri Slaby, kgdb-bugreport,
	linux-serial, linux-kernel
In-Reply-To: <20180910232014.2642-1-labbott@redhat.com>

On Mon, Sep 10, 2018 at 04:20:14PM -0700, Laura Abbott wrote:
> There's an error when compiled with restrict:
> 
> drivers/tty/serial/kgdboc.c: In function ‘configure_kgdboc’:
> drivers/tty/serial/kgdboc.c:137:2: error: ‘strcpy’ source argument is the same
> as destination [-Werror=restrict]
>   strcpy(config, opt);
>   ^~~~~~~~~~~~~~~~~~~
> 
> As the error implies, this is from trying to use config as both source and
> destination. Drop the call to the function where config is the argument
> since nothing else happens in the function.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
>  drivers/tty/serial/kgdboc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index b4ba2b1dab76..dd503743bbff 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -154,15 +154,13 @@ static int configure_kgdboc(void)
>  {
>  	struct tty_driver *p;
>  	int tty_line = 0;
> -	int err;
> +	int err = -ENODEV;
>  	char *cptr = config;
>  	struct console *cons;
>  
> -	err = kgdboc_option_setup(config);
> -	if (err || !strlen(config) || isspace(config[0]))
> +	if (!strlen(config) || isspace(config[0]))
>  		goto noconfig;
>  
> -	err = -ENODEV;
>  	kgdboc_io_ops.is_console = 0;
>  	kgdb_tty_driver = NULL;
>  
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [GIT PULL v2] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Greg KH @ 2018-09-11 14:04 UTC (permalink / raw)
  To: Lee Jones
  Cc: Radu Pirea, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, broonie, jslaby, richard.genoud, davem,
	mchehab+samsung, akpm, arnd, devicetree, linux-arm-kernel,
	linux-kernel, linux-serial, linux-spi
In-Reply-To: <20180911111850.GJ4185@dell>

On Tue, Sep 11, 2018 at 12:18:50PM +0100, Lee Jones wrote:
> [In case you missed it]
> 
> This new pull-request contains the following changes since v1:
> 
>  - Uplift the submission version from v11 to v12
>  - Include fix for build breakage due to missing Device Tree include file
> 
> Since it is based on the original pull-request, it can be pulled
> in either directly on top of it or as a pull in its own right.
> 
> The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> 
>   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20-1

Ok, let's try this again, I've dropped that other patch and taken this
branch into my tree.

Hopefully this all works well :)

thanks,

greg k-h

^ permalink raw reply

* Re: [GIT PULL v2] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Lee Jones @ 2018-09-11 14:07 UTC (permalink / raw)
  To: Greg KH
  Cc: Radu Pirea, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, broonie, jslaby, richard.genoud, davem,
	mchehab+samsung, akpm, arnd, devicetree, linux-arm-kernel,
	linux-kernel, linux-serial, linux-spi
In-Reply-To: <20180911140400.GB18410@kroah.com>

On Tue, 11 Sep 2018, Greg KH wrote:

> On Tue, Sep 11, 2018 at 12:18:50PM +0100, Lee Jones wrote:
> > [In case you missed it]
> > 
> > This new pull-request contains the following changes since v1:
> > 
> >  - Uplift the submission version from v11 to v12
> >  - Include fix for build breakage due to missing Device Tree include file
> > 
> > Since it is based on the original pull-request, it can be pulled
> > in either directly on top of it or as a pull in its own right.
> > 
> > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > 
> >   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > 
> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20-1
> 
> Ok, let's try this again, I've dropped that other patch and taken this
> branch into my tree.

What happened to not rebasing? :D

> Hopefully this all works well :)

Me too. :)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [GIT PULL v2] Immutable branch between MFD, SPI and TTY due for the v4.20 merge window
From: Greg KH @ 2018-09-11 14:12 UTC (permalink / raw)
  To: Lee Jones
  Cc: Radu Pirea, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, broonie, jslaby, richard.genoud, davem,
	mchehab+samsung, akpm, arnd, devicetree, linux-arm-kernel,
	linux-kernel, linux-serial, linux-spi
In-Reply-To: <20180911140733.GU4185@dell>

On Tue, Sep 11, 2018 at 03:07:33PM +0100, Lee Jones wrote:
> On Tue, 11 Sep 2018, Greg KH wrote:
> 
> > On Tue, Sep 11, 2018 at 12:18:50PM +0100, Lee Jones wrote:
> > > [In case you missed it]
> > > 
> > > This new pull-request contains the following changes since v1:
> > > 
> > >  - Uplift the submission version from v11 to v12
> > >  - Include fix for build breakage due to missing Device Tree include file
> > > 
> > > Since it is based on the original pull-request, it can be pulled
> > > in either directly on top of it or as a pull in its own right.
> > > 
> > > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
> > > 
> > >   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git tags/ib-mfd-spi-tty-v4.20-1
> > 
> > Ok, let's try this again, I've dropped that other patch and taken this
> > branch into my tree.
> 
> What happened to not rebasing? :D

Turned out I hadn't pushed to my immutable branch yet, we got lucky :)

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Geert Uytterhoeven @ 2018-09-11 14:59 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Lee Jones, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS,
	linux-spi <linux-spi@
In-Reply-To: <20180911093917.GL2494@piout.net>

Hi Alexandre,

On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > Radu Pirea (6):
> > >   MAINTAINERS: add at91 usart mfd driver
> > >   dt-bindings: add binding for atmel-usart in SPI mode
> > >   mfd: at91-usart: added mfd driver for usart
> > >   MAINTAINERS: add at91 usart spi driver
> > >   spi: at91-usart: add driver for at91-usart as spi
> > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > >
> > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > >  MAINTAINERS                                   |  16 +
> > >  drivers/mfd/Kconfig                           |   9 +
> > >  drivers/mfd/Makefile                          |   1 +
> > >  drivers/mfd/at91-usart.c                      |  71 +++
> > >  drivers/spi/Kconfig                           |   8 +
> > >  drivers/spi/Makefile                          |   1 +
> > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > >  drivers/tty/serial/Kconfig                    |   1 +
> > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > >  create mode 100644 drivers/mfd/at91-usart.c
> > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> >
> > Seeing as this patch-set has caused some issues this morning, I took
> > the liberty to peruse back into its history to figure out where things
> > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > I did!
> >
> > My Acked-by has been attached to the MFD portion since v5, which is
> > why the code hasn't caught my eye before today.  I reviewed the
> > relocation of the *binding document* (serial => mfd with no changes)
> > in v4 and nothing else.  It appears as though you mistakenly added it
> > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > told you I'd already reviewed the binding document.
> >
> > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > that I do not like what I see.  Besides the missing header file and
> > the whitespace tabbing errors, I do not agree with the implementation.
> > Using MFD as a shim to hack around driver selection is not a valid
> > use-case.
> >
> > What's stopping you from just using the compatible string directly to
> > select which driver you need to probe?
> >
>
> Then you'd have multiple compatible strings for the same IP which is a
> big no-no.

It's still the same hardware device, isn't?
What if the SPI or UART slave is not on-board, but on an expansion board?
Then the SoC-specific .dtsi has no idea what mode should be used.

Hence shouldn't the software derive the hardware mode from the full
hardware description in DT? If that's impossible (I didn't look into detail
whether an SPI bus can easily be distinguished from a UART bus), perhaps
a mode property should be added?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Alexandre Belloni @ 2018-09-11 15:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lee Jones, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS,
	linux-spi <linux-spi@
In-Reply-To: <CAMuHMdXH8gNjKGOHGFc+K03U2zsVCSN7G_Q4wtepQr859nRQKg@mail.gmail.com>

On 11/09/2018 16:59:09+0200, Geert Uytterhoeven wrote:
> Hi Alexandre,
> 
> On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > > Radu Pirea (6):
> > > >   MAINTAINERS: add at91 usart mfd driver
> > > >   dt-bindings: add binding for atmel-usart in SPI mode
> > > >   mfd: at91-usart: added mfd driver for usart
> > > >   MAINTAINERS: add at91 usart spi driver
> > > >   spi: at91-usart: add driver for at91-usart as spi
> > > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > > >
> > > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > > >  MAINTAINERS                                   |  16 +
> > > >  drivers/mfd/Kconfig                           |   9 +
> > > >  drivers/mfd/Makefile                          |   1 +
> > > >  drivers/mfd/at91-usart.c                      |  71 +++
> > > >  drivers/spi/Kconfig                           |   8 +
> > > >  drivers/spi/Makefile                          |   1 +
> > > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > > >  drivers/tty/serial/Kconfig                    |   1 +
> > > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > > >  create mode 100644 drivers/mfd/at91-usart.c
> > > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > >
> > > Seeing as this patch-set has caused some issues this morning, I took
> > > the liberty to peruse back into its history to figure out where things
> > > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > > I did!
> > >
> > > My Acked-by has been attached to the MFD portion since v5, which is
> > > why the code hasn't caught my eye before today.  I reviewed the
> > > relocation of the *binding document* (serial => mfd with no changes)
> > > in v4 and nothing else.  It appears as though you mistakenly added it
> > > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > > told you I'd already reviewed the binding document.
> > >
> > > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > > that I do not like what I see.  Besides the missing header file and
> > > the whitespace tabbing errors, I do not agree with the implementation.
> > > Using MFD as a shim to hack around driver selection is not a valid
> > > use-case.
> > >
> > > What's stopping you from just using the compatible string directly to
> > > select which driver you need to probe?
> > >
> >
> > Then you'd have multiple compatible strings for the same IP which is a
> > big no-no.
> 
> It's still the same hardware device, isn't?
> What if the SPI or UART slave is not on-board, but on an expansion board?
> Then the SoC-specific .dtsi has no idea what mode should be used.
> 
> Hence shouldn't the software derive the hardware mode from the full
> hardware description in DT? If that's impossible (I didn't look into detail
> whether an SPI bus can easily be distinguished from a UART bus), perhaps
> a mode property should be added?
> 

Yes, this is exactly what is done:

https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/tree/drivers/mfd/at91-usart.c?h=ib-mfd-spi-tty-4.20-1#n33

Only one compatbile for the IP and a property to know what is the mode.
That property should indeed be set in the board dts and not the SoC
dtsi.

the other, less robust alternative was to look for child nodes and
decide that if some where present it would indicate an SPI bus. But I
think at some point we may have child nodes under a UART node.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Geert Uytterhoeven @ 2018-09-11 16:23 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Lee Jones, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS,
	linux-spi <linux-spi@
In-Reply-To: <20180911153621.GP2494@piout.net>

On Tue, Sep 11, 2018 at 5:36 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 11/09/2018 16:59:09+0200, Geert Uytterhoeven wrote:
> > On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > > On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > > > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > > > Radu Pirea (6):
> > > > >   MAINTAINERS: add at91 usart mfd driver
> > > > >   dt-bindings: add binding for atmel-usart in SPI mode
> > > > >   mfd: at91-usart: added mfd driver for usart
> > > > >   MAINTAINERS: add at91 usart spi driver
> > > > >   spi: at91-usart: add driver for at91-usart as spi
> > > > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > > > >
> > > > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > > > >  MAINTAINERS                                   |  16 +
> > > > >  drivers/mfd/Kconfig                           |   9 +
> > > > >  drivers/mfd/Makefile                          |   1 +
> > > > >  drivers/mfd/at91-usart.c                      |  71 +++
> > > > >  drivers/spi/Kconfig                           |   8 +
> > > > >  drivers/spi/Makefile                          |   1 +
> > > > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > > > >  drivers/tty/serial/Kconfig                    |   1 +
> > > > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > > > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > > > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > > > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > > > >  create mode 100644 drivers/mfd/at91-usart.c
> > > > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > > > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > > >
> > > > Seeing as this patch-set has caused some issues this morning, I took
> > > > the liberty to peruse back into its history to figure out where things
> > > > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > > > I did!
> > > >
> > > > My Acked-by has been attached to the MFD portion since v5, which is
> > > > why the code hasn't caught my eye before today.  I reviewed the
> > > > relocation of the *binding document* (serial => mfd with no changes)
> > > > in v4 and nothing else.  It appears as though you mistakenly added it
> > > > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > > > told you I'd already reviewed the binding document.
> > > >
> > > > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > > > that I do not like what I see.  Besides the missing header file and
> > > > the whitespace tabbing errors, I do not agree with the implementation.
> > > > Using MFD as a shim to hack around driver selection is not a valid
> > > > use-case.
> > > >
> > > > What's stopping you from just using the compatible string directly to
> > > > select which driver you need to probe?
> > > >
> > >
> > > Then you'd have multiple compatible strings for the same IP which is a
> > > big no-no.
> >
> > It's still the same hardware device, isn't?
> > What if the SPI or UART slave is not on-board, but on an expansion board?
> > Then the SoC-specific .dtsi has no idea what mode should be used.
> >
> > Hence shouldn't the software derive the hardware mode from the full
> > hardware description in DT? If that's impossible (I didn't look into detail
> > whether an SPI bus can easily be distinguished from a UART bus), perhaps
> > a mode property should be added?
>
> Yes, this is exactly what is done:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/tree/drivers/mfd/at91-usart.c?h=ib-mfd-spi-tty-4.20-1#n33

OK.

I guess the main "hackish" part is that the mfd_cell uses of_compatible,
which thus requires having additional compatible values?

I think those can just be removed. AFAICS, the SPI and serial drivers already
match against the "at91_usart_spi" resp. "atmel_usart_serial" platform device
names?

> Only one compatbile for the IP and a property to know what is the mode.
> That property should indeed be set in the board dts and not the SoC
> dtsi.
>
> the other, less robust alternative was to look for child nodes and
> decide that if some where present it would indicate an SPI bus. But I
> think at some point we may have child nodes under a UART node.

Indeed, using the "new" serial bus.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Lee Jones @ 2018-09-11 18:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Alexandre Belloni, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS
In-Reply-To: <CAMuHMdV+=EgAYogwNHbY=RExkGox_+YSeHV6pJtQb6g6PzD-CQ@mail.gmail.com>

On Tue, 11 Sep 2018, Geert Uytterhoeven wrote:

> On Tue, Sep 11, 2018 at 5:36 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > On 11/09/2018 16:59:09+0200, Geert Uytterhoeven wrote:
> > > On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > > On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > > > > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > > > > Radu Pirea (6):
> > > > > >   MAINTAINERS: add at91 usart mfd driver
> > > > > >   dt-bindings: add binding for atmel-usart in SPI mode
> > > > > >   mfd: at91-usart: added mfd driver for usart
> > > > > >   MAINTAINERS: add at91 usart spi driver
> > > > > >   spi: at91-usart: add driver for at91-usart as spi
> > > > > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > > > > >
> > > > > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > > > > >  MAINTAINERS                                   |  16 +
> > > > > >  drivers/mfd/Kconfig                           |   9 +
> > > > > >  drivers/mfd/Makefile                          |   1 +
> > > > > >  drivers/mfd/at91-usart.c                      |  71 +++
> > > > > >  drivers/spi/Kconfig                           |   8 +
> > > > > >  drivers/spi/Makefile                          |   1 +
> > > > > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > > > > >  drivers/tty/serial/Kconfig                    |   1 +
> > > > > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > > > > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > > > > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > > > > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > > > > >  create mode 100644 drivers/mfd/at91-usart.c
> > > > > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > > > > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > > > >
> > > > > Seeing as this patch-set has caused some issues this morning, I took
> > > > > the liberty to peruse back into its history to figure out where things
> > > > > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > > > > I did!
> > > > >
> > > > > My Acked-by has been attached to the MFD portion since v5, which is
> > > > > why the code hasn't caught my eye before today.  I reviewed the
> > > > > relocation of the *binding document* (serial => mfd with no changes)
> > > > > in v4 and nothing else.  It appears as though you mistakenly added it
> > > > > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > > > > told you I'd already reviewed the binding document.
> > > > >
> > > > > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > > > > that I do not like what I see.  Besides the missing header file and
> > > > > the whitespace tabbing errors, I do not agree with the implementation.
> > > > > Using MFD as a shim to hack around driver selection is not a valid
> > > > > use-case.
> > > > >
> > > > > What's stopping you from just using the compatible string directly to
> > > > > select which driver you need to probe?
> > > > >
> > > >
> > > > Then you'd have multiple compatible strings for the same IP which is a
> > > > big no-no.
> > >
> > > It's still the same hardware device, isn't?
> > > What if the SPI or UART slave is not on-board, but on an expansion board?
> > > Then the SoC-specific .dtsi has no idea what mode should be used.
> > >
> > > Hence shouldn't the software derive the hardware mode from the full
> > > hardware description in DT? If that's impossible (I didn't look into detail
> > > whether an SPI bus can easily be distinguished from a UART bus), perhaps
> > > a mode property should be added?
> >
> > Yes, this is exactly what is done:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/tree/drivers/mfd/at91-usart.c?h=ib-mfd-spi-tty-4.20-1#n33
> 
> OK.
> 
> I guess the main "hackish" part is that the mfd_cell uses of_compatible,
> which thus requires having additional compatible values?
> 
> I think those can just be removed. AFAICS, the SPI and serial drivers already
> match against the "at91_usart_spi" resp. "atmel_usart_serial" platform device
> names?

The hackish part of this driver is that it's using MFD for something
which is clearly not an MFD.  It's a USART device.  Nothing more,
nothing less.

Does anyone have the datasheet to hand?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Alexandre Belloni @ 2018-09-11 18:58 UTC (permalink / raw)
  To: Lee Jones
  Cc: Geert Uytterhoeven, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS
In-Reply-To: <20180911181838.GI4185@dell>

On 11/09/2018 19:39:30+0100, Lee Jones wrote:
> On Tue, 11 Sep 2018, Geert Uytterhoeven wrote:
> 
> > On Tue, Sep 11, 2018 at 5:36 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > > On 11/09/2018 16:59:09+0200, Geert Uytterhoeven wrote:
> > > > On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
> > > > <alexandre.belloni@bootlin.com> wrote:
> > > > > On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > > > > > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > > > > > Radu Pirea (6):
> > > > > > >   MAINTAINERS: add at91 usart mfd driver
> > > > > > >   dt-bindings: add binding for atmel-usart in SPI mode
> > > > > > >   mfd: at91-usart: added mfd driver for usart
> > > > > > >   MAINTAINERS: add at91 usart spi driver
> > > > > > >   spi: at91-usart: add driver for at91-usart as spi
> > > > > > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > > > > > >
> > > > > > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > > > > > >  MAINTAINERS                                   |  16 +
> > > > > > >  drivers/mfd/Kconfig                           |   9 +
> > > > > > >  drivers/mfd/Makefile                          |   1 +
> > > > > > >  drivers/mfd/at91-usart.c                      |  71 +++
> > > > > > >  drivers/spi/Kconfig                           |   8 +
> > > > > > >  drivers/spi/Makefile                          |   1 +
> > > > > > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > > > > > >  drivers/tty/serial/Kconfig                    |   1 +
> > > > > > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > > > > > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > > > > > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > > > > > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > > > > > >  create mode 100644 drivers/mfd/at91-usart.c
> > > > > > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > > > > > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > > > > >
> > > > > > Seeing as this patch-set has caused some issues this morning, I took
> > > > > > the liberty to peruse back into its history to figure out where things
> > > > > > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > > > > > I did!
> > > > > >
> > > > > > My Acked-by has been attached to the MFD portion since v5, which is
> > > > > > why the code hasn't caught my eye before today.  I reviewed the
> > > > > > relocation of the *binding document* (serial => mfd with no changes)
> > > > > > in v4 and nothing else.  It appears as though you mistakenly added it
> > > > > > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > > > > > told you I'd already reviewed the binding document.
> > > > > >
> > > > > > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > > > > > that I do not like what I see.  Besides the missing header file and
> > > > > > the whitespace tabbing errors, I do not agree with the implementation.
> > > > > > Using MFD as a shim to hack around driver selection is not a valid
> > > > > > use-case.
> > > > > >
> > > > > > What's stopping you from just using the compatible string directly to
> > > > > > select which driver you need to probe?
> > > > > >
> > > > >
> > > > > Then you'd have multiple compatible strings for the same IP which is a
> > > > > big no-no.
> > > >
> > > > It's still the same hardware device, isn't?
> > > > What if the SPI or UART slave is not on-board, but on an expansion board?
> > > > Then the SoC-specific .dtsi has no idea what mode should be used.
> > > >
> > > > Hence shouldn't the software derive the hardware mode from the full
> > > > hardware description in DT? If that's impossible (I didn't look into detail
> > > > whether an SPI bus can easily be distinguished from a UART bus), perhaps
> > > > a mode property should be added?
> > >
> > > Yes, this is exactly what is done:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/tree/drivers/mfd/at91-usart.c?h=ib-mfd-spi-tty-4.20-1#n33
> > 
> > OK.
> > 
> > I guess the main "hackish" part is that the mfd_cell uses of_compatible,
> > which thus requires having additional compatible values?
> > 
> > I think those can just be removed. AFAICS, the SPI and serial drivers already
> > match against the "at91_usart_spi" resp. "atmel_usart_serial" platform device
> > names?
> 
> The hackish part of this driver is that it's using MFD for something
> which is clearly not an MFD.  It's a USART device.  Nothing more,
> nothing less.
> 
> Does anyone have the datasheet to hand?
> 

It is not a simple usart, it is either a usart or a full blown SPI
controller with registers changing layout depending on the selected
mode. Otherwise, I'm not sure how you would get a USART to do SPI.

Datasheet here:

http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6438-32-bit-ARM926-Embedded-Microprocessor-SAM9G45_Datasheet.pdf

USART doc starting p572, registers p621.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v12 0/6] Driver for at91 usart in spi mode
From: Geert Uytterhoeven @ 2018-09-11 18:59 UTC (permalink / raw)
  To: Lee Jones
  Cc: Alexandre Belloni, radu_nicolae.pirea, Rob Herring, Mark Rutland,
	Nicolas Ferre, Greg KH, Mark Brown, Jiri Slaby, Richard Genoud,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Arnd Bergmann,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, open list:SERIAL DRIVERS
In-Reply-To: <20180911181838.GI4185@dell>

Hi Lee,

On Tue, Sep 11, 2018 at 8:40 PM Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 11 Sep 2018, Geert Uytterhoeven wrote:
> > On Tue, Sep 11, 2018 at 5:36 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > > On 11/09/2018 16:59:09+0200, Geert Uytterhoeven wrote:
> > > > On Tue, Sep 11, 2018 at 11:40 AM Alexandre Belloni
> > > > <alexandre.belloni@bootlin.com> wrote:
> > > > > On 11/09/2018 10:33:56+0100, Lee Jones wrote:
> > > > > > On Tue, 04 Sep 2018, Radu Pirea wrote:
> > > > > > > Radu Pirea (6):
> > > > > > >   MAINTAINERS: add at91 usart mfd driver
> > > > > > >   dt-bindings: add binding for atmel-usart in SPI mode
> > > > > > >   mfd: at91-usart: added mfd driver for usart
> > > > > > >   MAINTAINERS: add at91 usart spi driver
> > > > > > >   spi: at91-usart: add driver for at91-usart as spi
> > > > > > >   tty/serial: atmel: change the driver to work under at91-usart mfd
> > > > > > >
> > > > > > >  .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
> > > > > > >  MAINTAINERS                                   |  16 +
> > > > > > >  drivers/mfd/Kconfig                           |   9 +
> > > > > > >  drivers/mfd/Makefile                          |   1 +
> > > > > > >  drivers/mfd/at91-usart.c                      |  71 +++
> > > > > > >  drivers/spi/Kconfig                           |   8 +
> > > > > > >  drivers/spi/Makefile                          |   1 +
> > > > > > >  drivers/spi/spi-at91-usart.c                  | 432 ++++++++++++++++++
> > > > > > >  drivers/tty/serial/Kconfig                    |   1 +
> > > > > > >  drivers/tty/serial/atmel_serial.c             |  42 +-
> > > > > > >  include/dt-bindings/mfd/at91-usart.h          |  17 +
> > > > > > >  11 files changed, 606 insertions(+), 17 deletions(-)
> > > > > > >  rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> > > > > > >  create mode 100644 drivers/mfd/at91-usart.c
> > > > > > >  create mode 100644 drivers/spi/spi-at91-usart.c
> > > > > > >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > > > > >
> > > > > > Seeing as this patch-set has caused some issues this morning, I took
> > > > > > the liberty to peruse back into its history to figure out where things
> > > > > > started to go wrong.  I also re-reviewed the MFD driver - and I'm glad
> > > > > > I did!
> > > > > >
> > > > > > My Acked-by has been attached to the MFD portion since v5, which is
> > > > > > why the code hasn't caught my eye before today.  I reviewed the
> > > > > > relocation of the *binding document* (serial => mfd with no changes)
> > > > > > in v4 and nothing else.  It appears as though you mistakenly added it
> > > > > > to the *MFD driver* instead.  This explains my confusion in v10 when I
> > > > > > told you I'd already reviewed the binding document.
> > > > > >
> > > > > > As I said, I have re-reviewed the MFD driver and I'm afraid to say
> > > > > > that I do not like what I see.  Besides the missing header file and
> > > > > > the whitespace tabbing errors, I do not agree with the implementation.
> > > > > > Using MFD as a shim to hack around driver selection is not a valid
> > > > > > use-case.
> > > > > >
> > > > > > What's stopping you from just using the compatible string directly to
> > > > > > select which driver you need to probe?
> > > > > >
> > > > >
> > > > > Then you'd have multiple compatible strings for the same IP which is a
> > > > > big no-no.
> > > >
> > > > It's still the same hardware device, isn't?
> > > > What if the SPI or UART slave is not on-board, but on an expansion board?
> > > > Then the SoC-specific .dtsi has no idea what mode should be used.
> > > >
> > > > Hence shouldn't the software derive the hardware mode from the full
> > > > hardware description in DT? If that's impossible (I didn't look into detail
> > > > whether an SPI bus can easily be distinguished from a UART bus), perhaps
> > > > a mode property should be added?
> > >
> > > Yes, this is exactly what is done:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/tree/drivers/mfd/at91-usart.c?h=ib-mfd-spi-tty-4.20-1#n33
> >
> > OK.
> >
> > I guess the main "hackish" part is that the mfd_cell uses of_compatible,
> > which thus requires having additional compatible values?
> >
> > I think those can just be removed. AFAICS, the SPI and serial drivers already
> > match against the "at91_usart_spi" resp. "atmel_usart_serial" platform device
> > names?
>
> The hackish part of this driver is that it's using MFD for something
> which is clearly not an MFD.  It's a USART device.  Nothing more,
> nothing less.
>
> Does anyone have the datasheet to hand?

I haven't read it, but I believe it's not unlike Renesas SCIF, which is
served by both drivers/tty/serial/sh-sci.c and drivers/spi/spi-sh-sci.c.
But the latter is not used from DT, so we haven't experienced (and solved)
the similar issue yet.

Would it work if the UART driver and SPI driver would match against the
same compatible value, but the UART driver would do in its probe()
function:

    device_property_read_u32(&pdev->dev, "atmel,usart-mode", &opmode);
    if (opmode != AT91_USART_MODE_SERIAL)
        return ENODEV;

while the SPI driver would do:

    device_property_read_u32(&pdev->dev, "atmel,usart-mode", &opmode);
    if (opmode != AT91_USART_MODE_SPI)
        return ENODEV;

? No MFD driver involved.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox