Re: [PATCH v1 2/3] dt-bindings: soc: samsung: usi: add USIv1 and samsung,exynos8895-usi

[Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>]

On 1/3/25 10:21, Krzysztof Kozlowski wrote:
> On Thu, Jan 02, 2025 at 10:40:14PM +0200, Ivaylo Ivanov wrote:
>> Add constants for choosing USIv1 configuration mode in device tree.
>> Those are further used in the USI driver to figure out which value to
>> write into SW_CONF register. Modify the current USI IP-core
>> bindings to include information about USIv1 and a compatible for
>> exynos8895.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  .../bindings/soc/samsung/exynos-usi.yaml      | 22 ++++++++++++++-----
>>  include/dt-bindings/soc/samsung,exynos-usi.h  |  8 +++++++
>>  2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>> index f80fcbc31..50db63a8a 100644
>> --- a/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>> @@ -11,11 +11,21 @@ maintainers:
>>    - Krzysztof Kozlowski <krzk@kernel.org>
>>  
>>  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.
>> +  The USI IP-core provides configurable support for serial protocols, enabling
>> +  different serial communication modes depending on the version.
>> +
>> +  In USIv1, configurations are available to enable either one or two protocols
>> +  simultaneously in select combinations - High-Speed I2C0, High-Speed
>> +  I2C1, SPI, UART, High-Speed I2C0 and I2C1 or both High-Speed
>> +  I2C1 and UART.
>> +
>> +  In USIv2, only one protocol can be active at a time, either UART, SPI, or
>> +  High-Speed I2C.
>> +
>> +  The USI core shares internal circuits across protocols, meaning only the
>> +  selected configuration is active at any given 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.
>>  
>>  properties:
>>    $nodename:
>> @@ -31,6 +41,7 @@ properties:
>>            - const: samsung,exynos850-usi
>>        - enum:
>>            - samsung,exynos850-usi
>> +          - samsung,exynos8895-usi
>>  
>>    reg:
>>      maxItems: 1
>> @@ -106,6 +117,7 @@ if:
>>        contains:
>>          enum:
>>            - samsung,exynos850-usi
>> +          - samsung,exynos8895-usi
>>  
>>  then:
>>    properties:
>> diff --git a/include/dt-bindings/soc/samsung,exynos-usi.h b/include/dt-bindings/soc/samsung,exynos-usi.h
>> index a01af169d..5fbeb426b 100644
>> --- a/include/dt-bindings/soc/samsung,exynos-usi.h
>> +++ b/include/dt-bindings/soc/samsung,exynos-usi.h
>> @@ -9,6 +9,14 @@
>>  #ifndef __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H
>>  #define __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H
>>  
>> +#define USI_V1_NONE		0
>> +#define USI_V1_I2C0		1
>> +#define USI_V1_I2C1		2
>> +#define USI_V1_I2C0_1		3
>> +#define USI_V1_SPI		4
>> +#define USI_V1_UART		5
>> +#define USI_V1_UART_I2C1	6
> Conflicting values with USI_v2. This is the same property and the same
> driver, so I expect the values to be unified.
>
> Rename (copy) USI_V2_ to USI_ and then just add yours at the end.

I see, that approach could work too. Although I'd much rather keep the definition
names the same as before for USI_V2.

Best regards,
Ivaylo

> Binding then in allOf:if:then should restrict with enum which values
> make sense for which model.
>
> Best regards,
> Krzysztof
>