From:   "Chanho Park" <chanho61.park@samsung.com>
To:     "'Krzysztof Kozlowski'" <krzysztof.kozlowski@canonical.com>,
        "'Arnd Bergmann'" <arnd@arndb.de>,
        "'Olof Johansson'" <olof@lixom.net>
Cc:     "'Jaewon Kim'" <jaewon02.kim@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>
In-Reply-To: <14731ee8-bc9a-8973-2bcf-e7a771ac3c07@canonical.com>
Subject: RE: [PATCH v3 0/5] soc: samsung: Add USI driver
Date:   Wed, 8 Dec 2021 18:13:03 +0900
Message-ID: <02a001d7ec13$ce088e80$6a19ab80$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQJqUxbh1QCt+OxulIgHHWgadF9h5gF5SxqWAadkv+QCbflt2arXCXkA
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <20211204195757.8600-1-semen.protsenko@linaro.org>
        <fb31a159-6d2e-6c9a-439f-f19ef4fd4732@canonical.com>
        <CGME20211206182351epcas2p4b381b620bb0f7f7928857b7003f7e83c@epcas2p4.samsung.com>
        <14731ee8-bc9a-8973-2bcf-e7a771ac3c07@canonical.com>
Precedence: bulk

Hi,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski=40canonical.com>
> Sent: Tuesday, December 7, 2021 3:24 AM
> To: Arnd Bergmann <arnd=40arndb.de>; Olof Johansson <olof=40lixom.net>
> Cc: Jaewon Kim <jaewon02.kim=40samsung.com>; Chanho Park
> <chanho61.park=40samsung.com>; David Virag <virag.david003=40gmail.com>;
> Youngmin Nam <youngmin.nam=40samsung.com>; Sam Protsenko
> <semen.protsenko=40linaro.org>; devicetree=40vger.kernel.org; linux-
> serial=40vger.kernel.org; linux-arm-kernel=40lists.infradead.org; Greg Kr=
oah-
> Hartman <gregkh=40linuxfoundation.org>; linux-kernel=40vger.kernel.org; l=
inux-
> samsung-soc=40vger.kernel.org; Rob Herring <robh+dt=40kernel.org>
> Subject: Re: =5BPATCH v3 0/5=5D soc: samsung: Add USI driver
>=20
> 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:
> >>   - =22reg=22 property contains USI registers start address (0xc0 offs=
et);
> >>     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 =22samsung,sysreg=22 property; it's used to
> >>     select USI function (protocol to be used)
> >>   - USI function is specified in =22samsung,mode=22 property; integer =
value
> >>     is used to simplify parsing
> >>   - there is =22samsung,clkreq-on=22 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 drive=
r
> >>       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: =22tty: serial: samsung: Fix console registration fro=
m
> >>     module=22
> >>   - 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.
>=20
> +CC Arnd and Olof,
>=20
> Dear Arnd and Olof,
>=20
> 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.
>=20
> The bisectability and ABI break will affect only newly upstreamed Samsung
> Exynos SoC, so for now only Exynos Auto v9.
>=20
> 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 mak=
e
> 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 ne=
w.
> 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 ar=
e
> not a base for Samsung vendor kernels.

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

Best Regards,
Chanho Park