mbox series

[v3,0/5] soc: samsung: Add USI driver

Message ID 20211204195757.8600-1-semen.protsenko@linaro.org
Headers show
Series soc: samsung: Add USI driver | expand

Message

Sam Protsenko Dec. 4, 2021, 7:57 p.m. UTC
USIv2 IP-core provides selectable serial protocol (UART, SPI or
High-Speed I2C); only one can be chosen at a time. This series
implements USIv2 driver, which allows one to select particular USI
function in device tree, and also performs USI block initialization.

With that driver implemented, it's not needed to do USI initialization
in protocol drivers anymore, so that code is removed from the serial
driver.

Because USI driver is tristate (can be built as a module), serial driver
was reworked so it's possible to use its console part as a module too.
This way we can load serial driver module from user space and still have
serial console functional.

Design features:
  - "reg" property contains USI registers start address (0xc0 offset);
    it's used in the driver to access USI_CON and USI_OPTION registers.
    This way all USI initialization (reset, HWACG, etc) can be done in
    USIv2 driver separately, rather than duplicating that code over
    UART/SPI/I2C drivers
  - System Register (system controller node) and its SW_CONF register
    offset are provided in "samsung,sysreg" property; it's used to
    select USI function (protocol to be used)
  - USI function is specified in "samsung,mode" property; integer value
    is used to simplify parsing
  - there is "samsung,clkreq-on" bool property, which makes driver
    disable HWACG control (needed for UART to work properly)
  - PCLK and IPCLK clocks are both provided to USI node; apparently both
    need to be enabled to access USI registers
  - protocol nodes are embedded (as a child nodes) in USI node; it
    allows correct init order, and reflects HW properly
  - USI driver is a tristate: can be also useful from Android GKI
    requirements point of view
  - driver functions are implemented with further development in mind:
    - we might want to add some DebugFs interface later
    - some functions might need to be revealed to serial drivers with
      EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
      instance
    - another USI revisions could be added (like USIv1)

Changes in v3:
  - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
  - Used clk_bulk API instead of handling each clock separately
  - Spell check fixes and coding style fixes
  - Improved dt-bindings doc

Changes in v2:
  - Renamed all 'usi_v2' wording to just 'usi' everywhere
  - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
    drivers
  - Added patch: "tty: serial: samsung: Fix console registration from
    module"
  - Combined dt-bindings doc and dt-bindings header patches
  - Reworked USI driver to be ready for USIv1 addition
  - Improved dt-bindings
  - Added USI_V2_NONE mode value

Sam Protsenko (5):
  dt-bindings: soc: samsung: Add Exynos USI bindings
  soc: samsung: Add USI driver
  tty: serial: samsung: Remove USI initialization
  tty: serial: samsung: Enable console as module
  tty: serial: samsung: Fix console registration from module

 .../bindings/soc/samsung/exynos-usi.yaml      | 159 ++++++++++
 drivers/soc/samsung/Kconfig                   |  14 +
 drivers/soc/samsung/Makefile                  |   2 +
 drivers/soc/samsung/exynos-usi.c              | 285 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   2 +-
 drivers/tty/serial/samsung_tty.c              |  78 ++---
 include/dt-bindings/soc/samsung,exynos-usi.h  |  17 ++
 include/linux/serial_s3c.h                    |   9 -
 8 files changed, 518 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
 create mode 100644 drivers/soc/samsung/exynos-usi.c
 create mode 100644 include/dt-bindings/soc/samsung,exynos-usi.h

Comments

Krzysztof Kozlowski Dec. 6, 2021, 6:23 p.m. UTC | #1
On 05/12/2021 17:24, Krzysztof Kozlowski wrote:
> On 04/12/2021 20:57, Sam Protsenko wrote:
>> USIv2 IP-core provides selectable serial protocol (UART, SPI or
>> High-Speed I2C); only one can be chosen at a time. This series
>> implements USIv2 driver, which allows one to select particular USI
>> function in device tree, and also performs USI block initialization.
>>
>> With that driver implemented, it's not needed to do USI initialization
>> in protocol drivers anymore, so that code is removed from the serial
>> driver.
>>
>> Because USI driver is tristate (can be built as a module), serial driver
>> was reworked so it's possible to use its console part as a module too.
>> This way we can load serial driver module from user space and still have
>> serial console functional.
>>
>> Design features:
>>   - "reg" property contains USI registers start address (0xc0 offset);
>>     it's used in the driver to access USI_CON and USI_OPTION registers.
>>     This way all USI initialization (reset, HWACG, etc) can be done in
>>     USIv2 driver separately, rather than duplicating that code over
>>     UART/SPI/I2C drivers
>>   - System Register (system controller node) and its SW_CONF register
>>     offset are provided in "samsung,sysreg" property; it's used to
>>     select USI function (protocol to be used)
>>   - USI function is specified in "samsung,mode" property; integer value
>>     is used to simplify parsing
>>   - there is "samsung,clkreq-on" bool property, which makes driver
>>     disable HWACG control (needed for UART to work properly)
>>   - PCLK and IPCLK clocks are both provided to USI node; apparently both
>>     need to be enabled to access USI registers
>>   - protocol nodes are embedded (as a child nodes) in USI node; it
>>     allows correct init order, and reflects HW properly
>>   - USI driver is a tristate: can be also useful from Android GKI
>>     requirements point of view
>>   - driver functions are implemented with further development in mind:
>>     - we might want to add some DebugFs interface later
>>     - some functions might need to be revealed to serial drivers with
>>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
>>       instance
>>     - another USI revisions could be added (like USIv1)
>>
>> Changes in v3:
>>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>>   - Used clk_bulk API instead of handling each clock separately
>>   - Spell check fixes and coding style fixes
>>   - Improved dt-bindings doc
>>
>> Changes in v2:
>>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
>>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
>>     drivers
>>   - Added patch: "tty: serial: samsung: Fix console registration from
>>     module"
>>   - Combined dt-bindings doc and dt-bindings header patches
>>   - Reworked USI driver to be ready for USIv1 addition
>>   - Improved dt-bindings
>>   - Added USI_V2_NONE mode value
>>
>> Sam Protsenko (5):
>>   dt-bindings: soc: samsung: Add Exynos USI bindings
>>   soc: samsung: Add USI driver
>>   tty: serial: samsung: Remove USI initialization
>>   tty: serial: samsung: Enable console as module
>>   tty: serial: samsung: Fix console registration from module
>>
> 
> All this looks good to me. The serial driver changes should come
> together with this one (usi driver is now a dependency for them). If I
> am correct, mention this please in future cover letter (if there is such).
> 
> I will still need DTSI changes for Exynos Auto v9 and confirmation that
> is not being used downstream and breaking DTB ABI is okay. Because this
> will be a non-bisctable and also a DTB ABI break.

+CC Arnd and Olof,

Dear Arnd and Olof,

The patchset discussed here reworks recently added USI code to Samsung
Exynos UART driver in a non-bisectable and ABI-breaking way. The
existing code in serial driver was added in v5.15-rc1, however first
user - Exyons Auto v9 - appeared in v5.16-rc1.

The bisectability and ABI break will affect only newly upstreamed
Samsung Exynos SoC, so for now only Exynos Auto v9.

The early code has some drawbacks and limitations which came up now when
we want to extend the USI code to support more blocks (I2C, SPI) and
devices (including older Exynos chipsets). Therefore I am planning to
make an ABI break of this features because:
1. The code was added recently (v5.15-rc1) and users even later (v5.16-rc1).
2. Even though code was merged, I consider it still development phase.
Kernel development goes very fast and we do not defer patches waiting
for perfect solution.
3. There are no known out-of-tree users of this because this is fairly
new. For this I am waiting for confirmation from Chanho (and/or other
Samsung folks). I don't expect there are out of tree users because any
mobile or automotive product will take Samsung vendor sources and recent
kernels are not a base for Samsung vendor kernels.
4. Keeping it backwards compatible will be a pain.

The alternative is to keep old code in Samsung Serial driver and try to
detect the case when driver is probed in old way (with old DTB). This
won't be trivial, because there will be no properties change, but
devicetree hierarchy change like:

from:
soc@0 {
  serial@0xabcd0123 {
    ...
  }
}

into:
soc@0 {
  usi@0x12345678 {
    serial@0xabcd0123 {
      ...
    }
  }
}


Are you okay with such planned ABI break?

Best regards,
Krzysztof
Chanho Park Dec. 8, 2021, 9:13 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Tuesday, December 7, 2021 3:24 AM
> To: Arnd Bergmann <arnd@arndb.de>; Olof Johansson <olof@lixom.net>
> Cc: Jaewon Kim <jaewon02.kim@samsung.com>; Chanho Park
> <chanho61.park@samsung.com>; David Virag <virag.david003@gmail.com>;
> Youngmin Nam <youngmin.nam@samsung.com>; Sam Protsenko
> <semen.protsenko@linaro.org>; devicetree@vger.kernel.org; linux-
> serial@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Greg Kroah-
> Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; linux-
> samsung-soc@vger.kernel.org; Rob Herring <robh+dt@kernel.org>
> Subject: Re: [PATCH v3 0/5] soc: samsung: Add USI driver
> 
> On 05/12/2021 17:24, Krzysztof Kozlowski wrote:
> > On 04/12/2021 20:57, Sam Protsenko wrote:
> >> USIv2 IP-core provides selectable serial protocol (UART, SPI or
> >> High-Speed I2C); only one can be chosen at a time. This series
> >> implements USIv2 driver, which allows one to select particular USI
> >> function in device tree, and also performs USI block initialization.
> >>
> >> With that driver implemented, it's not needed to do USI
> >> initialization in protocol drivers anymore, so that code is removed
> >> from the serial driver.
> >>
> >> Because USI driver is tristate (can be built as a module), serial
> >> driver was reworked so it's possible to use its console part as a
> module too.
> >> This way we can load serial driver module from user space and still
> >> have serial console functional.
> >>
> >> Design features:
> >>   - "reg" property contains USI registers start address (0xc0 offset);
> >>     it's used in the driver to access USI_CON and USI_OPTION registers.
> >>     This way all USI initialization (reset, HWACG, etc) can be done in
> >>     USIv2 driver separately, rather than duplicating that code over
> >>     UART/SPI/I2C drivers
> >>   - System Register (system controller node) and its SW_CONF register
> >>     offset are provided in "samsung,sysreg" property; it's used to
> >>     select USI function (protocol to be used)
> >>   - USI function is specified in "samsung,mode" property; integer value
> >>     is used to simplify parsing
> >>   - there is "samsung,clkreq-on" bool property, which makes driver
> >>     disable HWACG control (needed for UART to work properly)
> >>   - PCLK and IPCLK clocks are both provided to USI node; apparently
> both
> >>     need to be enabled to access USI registers
> >>   - protocol nodes are embedded (as a child nodes) in USI node; it
> >>     allows correct init order, and reflects HW properly
> >>   - USI driver is a tristate: can be also useful from Android GKI
> >>     requirements point of view
> >>   - driver functions are implemented with further development in mind:
> >>     - we might want to add some DebugFs interface later
> >>     - some functions might need to be revealed to serial drivers with
> >>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
> >>       instance
> >>     - another USI revisions could be added (like USIv1)
> >>
> >> Changes in v3:
> >>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-
> usi
> >>   - Used clk_bulk API instead of handling each clock separately
> >>   - Spell check fixes and coding style fixes
> >>   - Improved dt-bindings doc
> >>
> >> Changes in v2:
> >>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
> >>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
> >>     drivers
> >>   - Added patch: "tty: serial: samsung: Fix console registration from
> >>     module"
> >>   - Combined dt-bindings doc and dt-bindings header patches
> >>   - Reworked USI driver to be ready for USIv1 addition
> >>   - Improved dt-bindings
> >>   - Added USI_V2_NONE mode value
> >>
> >> Sam Protsenko (5):
> >>   dt-bindings: soc: samsung: Add Exynos USI bindings
> >>   soc: samsung: Add USI driver
> >>   tty: serial: samsung: Remove USI initialization
> >>   tty: serial: samsung: Enable console as module
> >>   tty: serial: samsung: Fix console registration from module
> >>
> >
> > All this looks good to me. The serial driver changes should come
> > together with this one (usi driver is now a dependency for them). If I
> > am correct, mention this please in future cover letter (if there is
> such).
> >
> > I will still need DTSI changes for Exynos Auto v9 and confirmation
> > that is not being used downstream and breaking DTB ABI is okay.
> > Because this will be a non-bisctable and also a DTB ABI break.
> 
> +CC Arnd and Olof,
> 
> Dear Arnd and Olof,
> 
> The patchset discussed here reworks recently added USI code to Samsung
> Exynos UART driver in a non-bisectable and ABI-breaking way. The existing
> code in serial driver was added in v5.15-rc1, however first user - Exyons
> Auto v9 - appeared in v5.16-rc1.
> 
> The bisectability and ABI break will affect only newly upstreamed Samsung
> Exynos SoC, so for now only Exynos Auto v9.
> 
> The early code has some drawbacks and limitations which came up now when
> we want to extend the USI code to support more blocks (I2C, SPI) and
> devices (including older Exynos chipsets). Therefore I am planning to make
> an ABI break of this features because:
> 1. The code was added recently (v5.15-rc1) and users even later (v5.16-
> rc1).
> 2. Even though code was merged, I consider it still development phase.
> Kernel development goes very fast and we do not defer patches waiting for
> perfect solution.
> 3. There are no known out-of-tree users of this because this is fairly new.
> For this I am waiting for confirmation from Chanho (and/or other Samsung
> folks). I don't expect there are out of tree users because any mobile or
> automotive product will take Samsung vendor sources and recent kernels are
> not a base for Samsung vendor kernels.

This will be not an issue in my case. Please apply below patch with this series.
https://lore.kernel.org/linux-samsung-soc/20211208003946.139423-1-chanho61.park@samsung.com/T/#u

Best Regards,
Chanho Park
Chanho Park Dec. 8, 2021, 9:15 a.m. UTC | #3
Hi,

> -----Original Message-----
> From: Sam Protsenko <semen.protsenko@linaro.org>
> Sent: Sunday, December 5, 2021 4:58 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; Rob Herring
> <robh+dt@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jaewon Kim <jaewon02.kim@samsung.com>; Chanho Park
> <chanho61.park@samsung.com>; David Virag <virag.david003@gmail.com>;
> Youngmin Nam <youngmin.nam@samsung.com>; devicetree@vger.kernel.org;
> linux-serial@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: [PATCH v3 0/5] soc: samsung: Add USI driver
> 
> USIv2 IP-core provides selectable serial protocol (UART, SPI or High-Speed
> I2C); only one can be chosen at a time. This series implements USIv2
> driver, which allows one to select particular USI function in device tree,
> and also performs USI block initialization.
> 
> With that driver implemented, it's not needed to do USI initialization in
> protocol drivers anymore, so that code is removed from the serial driver.
> 
> Because USI driver is tristate (can be built as a module), serial driver
> was reworked so it's possible to use its console part as a module too.
> This way we can load serial driver module from user space and still have
> serial console functional.
> 
> Design features:
>   - "reg" property contains USI registers start address (0xc0 offset);
>     it's used in the driver to access USI_CON and USI_OPTION registers.
>     This way all USI initialization (reset, HWACG, etc) can be done in
>     USIv2 driver separately, rather than duplicating that code over
>     UART/SPI/I2C drivers
>   - System Register (system controller node) and its SW_CONF register
>     offset are provided in "samsung,sysreg" property; it's used to
>     select USI function (protocol to be used)
>   - USI function is specified in "samsung,mode" property; integer value
>     is used to simplify parsing
>   - there is "samsung,clkreq-on" bool property, which makes driver
>     disable HWACG control (needed for UART to work properly)
>   - PCLK and IPCLK clocks are both provided to USI node; apparently both
>     need to be enabled to access USI registers
>   - protocol nodes are embedded (as a child nodes) in USI node; it
>     allows correct init order, and reflects HW properly
>   - USI driver is a tristate: can be also useful from Android GKI
>     requirements point of view
>   - driver functions are implemented with further development in mind:
>     - we might want to add some DebugFs interface later
>     - some functions might need to be revealed to serial drivers with
>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
>       instance
>     - another USI revisions could be added (like USIv1)
> 
> Changes in v3:
>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>   - Used clk_bulk API instead of handling each clock separately
>   - Spell check fixes and coding style fixes
>   - Improved dt-bindings doc
> 
> Changes in v2:
>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
>     drivers
>   - Added patch: "tty: serial: samsung: Fix console registration from
>     module"
>   - Combined dt-bindings doc and dt-bindings header patches
>   - Reworked USI driver to be ready for USIv1 addition
>   - Improved dt-bindings
>   - Added USI_V2_NONE mode value
> 
> Sam Protsenko (5):
>   dt-bindings: soc: samsung: Add Exynos USI bindings
>   soc: samsung: Add USI driver
>   tty: serial: samsung: Remove USI initialization
>   tty: serial: samsung: Enable console as module
>   tty: serial: samsung: Fix console registration from module

Tested-by: Chanho Park <chanho61.park@samsung.com> with below patch.
https://lore.kernel.org/linux-samsung-soc/20211208003946.139423-1-chanho61.p
ark@samsung.com/T/#u

Thanks.

Best Regards,
Chanho Park