On 08/01/2025 10:17, Ivaylo Ivanov wrote:
> On 1/8/25 10:30, Krzysztof Kozlowski wrote:
>> On Tue, Jan 07, 2025 at 01:35:11PM +0200, Ivaylo Ivanov wrote:
>>> USIv1 IP-core is found on some ARM64 Exynos SoCs (like Exynos8895) and
>>> provides selectable serial protocols (one of: HSI2C0, HSI2C1, HSI2C0_1,
>>> SPI, UART, UART_HSI2C1).
>>>
>>> USIv1, unlike USIv2, doesn't have any known register map. Underlying
>>> protocols that it implements have no offset, like with Exynos850.
>>> Desired protocol can be chosen via SW_CONF register from System
>>> Register block of the same domain as USI.
>>>
>>> In order to select a particular protocol, the protocol has to be
>>> selected via the System Register. Unlike USIv2, there's no need for
>>> any setup before the given protocol becomes accessible apart from
>>> enabling the APB clock and the protocol operating clock.
>>>
>>> Modify the existing driver in order to allow USIv1 instances in
>>> Exynos8895 to probe and set their protocol. While we're at it,
>>> make use of the new mode constants in place of the old ones
>>> and add a removal routine.
>>>
>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@xxxxxxxxx>
>>> ---
>>> drivers/soc/samsung/exynos-usi.c | 108 +++++++++++++++++++++++++++----
>>> 1 file changed, 95 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/soc/samsung/exynos-usi.c b/drivers/soc/samsung/exynos-usi.c
>>> index 114352695..43c17b100 100644
>>> --- a/drivers/soc/samsung/exynos-usi.c
>>> +++ b/drivers/soc/samsung/exynos-usi.c
>>> @@ -16,6 +16,18 @@
>>>
>>> #include <dt-bindings/soc/samsung,exynos-usi.h>
>>>
>>> +/* USIv1: System Register: SW_CONF register bits */
>>> +#define USI_V1_SW_CONF_NONE 0x0
>>> +#define USI_V1_SW_CONF_I2C0 0x1
>>> +#define USI_V1_SW_CONF_I2C1 0x2
>>> +#define USI_V1_SW_CONF_I2C0_1 0x3
>>> +#define USI_V1_SW_CONF_SPI 0x4
>>> +#define USI_V1_SW_CONF_UART 0x8
>>> +#define USI_V1_SW_CONF_UART_I2C1 0xa
>>> +#define USI_V1_SW_CONF_MASK (USI_V1_SW_CONF_I2C0 | USI_V1_SW_CONF_I2C1 | \
>>> + USI_V1_SW_CONF_I2C0_1 | USI_V1_SW_CONF_SPI | \
>>> + USI_V1_SW_CONF_UART | USI_V1_SW_CONF_UART_I2C1)
>>> +
>>> /* USIv2: System Register: SW_CONF register bits */
>>> #define USI_V2_SW_CONF_NONE 0x0
>>> #define USI_V2_SW_CONF_UART BIT(0)
>>> @@ -34,7 +46,8 @@
>>> #define USI_OPTION_CLKSTOP_ON BIT(2)
>>>
>>> enum exynos_usi_ver {
>>> - USI_VER2 = 2,
>>> + USI_VER1 = 1,
>> Is this assignment=1 actually now helping? Isn't it creating empty item
>> in exynos_usi_modes array? Basically it wastes space in the array for
>> no benefits.
>
> I wanted to keep the USIv2 enum the same.
Is there any need for keeping it the same?
>
>>
>>> + USI_VER2,
>>> };
...
>>
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void exynos_usi_remove(struct platform_device *pdev)
>>> +{
>>> + struct exynos_usi *usi = platform_get_drvdata(pdev);
>>> +
>>> + if (usi->data->ver == USI_VER2)
>>> + exynos_usi_disable(usi);
>> This is not related to the patch and should be separate patch, if at
>> all.
>
> Well I though that since didn't have any removal routine before it'd be good
> to introduce that and not leave USIv2 with hwacg set.
Sure, but separate commit, please. Can be preceeding the USIv1 support.
Best regards,
Krzysztof
