Message-ID: <8460edd6-efaf-3ae7-594c-0d1495d57abf@canonical.com>
Date:   Sat, 4 Dec 2021 12:28:14 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v2 RESEND 1/5] dt-bindings: soc: samsung: Add Exynos USI
 bindings
Content-Language: en-US
To:     Rob Herring <robh@kernel.org>,
        Sam Protsenko <semen.protsenko@linaro.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        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
References: <20211130111325.29328-1-semen.protsenko@linaro.org>
 <20211130111325.29328-2-semen.protsenko@linaro.org>
 <YaZ8BpUaaC+sJYqx@robh.at.kernel.org>
 <CAPLW+4kGGk+umKTVRPNM7R=GaUQa31Uid=K+9ofq8w2mqzGAEA@mail.gmail.com>
 <CAL_JsqLopqkOEWmnvMDWr2rBa5Dm3jf17soqVA=Jx5Hn9BDS_g@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
In-Reply-To: <CAL_JsqLopqkOEWmnvMDWr2rBa5Dm3jf17soqVA=Jx5Hn9BDS_g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/12/2021 21:40, Rob Herring wrote:
> On Fri, Dec 3, 2021 at 1:36 PM Sam Protsenko <semen.protsenko@linaro.org> wrote:
>>
>> On Tue, 30 Nov 2021 at 21:31, Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Tue, Nov 30, 2021 at 01:13:21PM +0200, Sam Protsenko wrote:
>>>> Add constants for choosing USIv2 configuration mode in device tree.
>>>> Those are further used in USI driver to figure out which value to write
>>>> into SW_CONF register. Also document USIv2 IP-core bindings.
>>>>
>>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>>>> ---
>>>> Changes in v2:
>>>>   - Combined dt-bindings doc and dt-bindings header patches
>>>>   - Added i2c node to example in bindings doc
>>>>   - Added mentioning of shared internal circuits
>>>>   - Added USI_V2_NONE value to bindings header
>>>>
>>>>  .../bindings/soc/samsung/exynos-usi.yaml      | 135 ++++++++++++++++++
>>>>  include/dt-bindings/soc/samsung,exynos-usi.h  |  17 +++
>>>>  2 files changed, 152 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>>>>  create mode 100644 include/dt-bindings/soc/samsung,exynos-usi.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>>>> new file mode 100644
>>>> index 000000000000..a822bc62b3cd
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>>>> @@ -0,0 +1,135 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/soc/samsung/exynos-usi.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Samsung's Exynos USI (Universal Serial Interface) binding
>>>> +
>>>> +maintainers:
>>>> +  - Sam Protsenko <semen.protsenko@linaro.org>
>>>> +  - Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>> +
>>>> +description: |
>>>> +  USI IP-core provides selectable serial protocol (UART, SPI or High-Speed I2C).
>>>> +  USI shares almost all internal circuits within each protocol, so only one
>>>> +  protocol can be chosen at a time. USI is modeled as a node with zero or more
>>>> +  child nodes, each representing a serial sub-node device. The mode setting
>>>> +  selects which particular function will be used.
>>>> +
>>>> +  Refer to next bindings documentation for information on protocol subnodes that
>>>> +  can exist under USI node:
>>>> +
>>>> +  [1] Documentation/devicetree/bindings/serial/samsung_uart.yaml
>>>> +  [2] Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>>>> +  [3] Documentation/devicetree/bindings/spi/spi-samsung.txt
>>>> +
>>>> +properties:
>>>> +  $nodename:
>>>> +    pattern: "^usi@[0-9a-f]+$"
>>>> +
>>>> +  compatible:
>>>> +    const: samsung,exynos-usi-v2
>>>
>>> Use SoC based compatibles.
>>>
>>
>> In this particular case, I'd really prefer to have it like this. Most
>> likely we'll only have USIv1 and USIv1 in the end, and I think that
>> would be more clear to have USI version in compatible, rather than SoC
>> name. Please let me know if you have a strong opinion on this one --
>> if so I'll re-send.
> 
> Fine if you have some evidence the ratio of versions to SoC are much
> more than 1:1 and the versions correspond to something (IOW, you
> aren't making them up).
> 
> We went down the version # path with QCom and in the end about every
> SoC had a different version.

I am against v1/v2 versions. The documentation in Samsung was always
poor in that matter. There were mistakes or confusions so it wasn't
always obvious which IP-block version comes with which SoC. Not
mentioning that several contributors do not have access to Samsung
datasheets and they submit code based on GPL compliance packages, so
they won't know which version they have for given SoC.

OTOH there is no single benefit of using USI v1/v2, except "liking".

Best regards,
Krzysztof