On Thu, Jan 02, 2025 at 10:40:15PM +0200, Ivaylo Ivanov wrote:
> /* 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,
> + USI_VER2,
> };
>
> struct exynos_usi_variant {
> @@ -66,6 +79,16 @@ struct exynos_usi_mode {
> unsigned int val; /* mode register value */
> };
>
> +static const struct exynos_usi_mode exynos_usi_v1_modes[] = {
> + [USI_V1_NONE] = { .name = "none", .val = USI_V1_SW_CONF_NONE },
> + [USI_V1_I2C0] = { .name = "i2c0", .val = USI_V1_SW_CONF_I2C0 },
> + [USI_V1_I2C1] = { .name = "i2c1", .val = USI_V1_SW_CONF_I2C1 },
> + [USI_V1_I2C0_1] = { .name = "i2c0_1", .val = USI_V1_SW_CONF_I2C0_1 },
> + [USI_V1_SPI] = { .name = "spi", .val = USI_V1_SW_CONF_SPI },
> + [USI_V1_UART] = { .name = "uart", .val = USI_V1_SW_CONF_UART },
> + [USI_V1_UART_I2C1] = { .name = "uart_i2c1", .val = USI_V1_SW_CONF_UART_I2C1 },
Now I see why you duplicated the IDs... With my approach your code here
is even simpler. Allows to drop USI_VER1 as well.
> +};
> +
> static const struct exynos_usi_mode exynos_usi_modes[] = {
> [USI_V2_NONE] = { .name = "none", .val = USI_V2_SW_CONF_NONE },
> [USI_V2_UART] = { .name = "uart", .val = USI_V2_SW_CONF_UART },
> @@ -83,11 +106,24 @@ static const struct exynos_usi_variant exynos850_usi_data = {
> .clk_names = exynos850_usi_clk_names,
> };
>
> +static const struct exynos_usi_variant exynos8895_usi_data = {
> + .ver = USI_VER1,
> + .sw_conf_mask = USI_V1_SW_CONF_MASK,
> + .min_mode = USI_V1_NONE,
> + .max_mode = USI_V1_UART_I2C1,
> + .num_clks = ARRAY_SIZE(exynos850_usi_clk_names),
> + .clk_names = exynos850_usi_clk_names,
> +};
> +
> static const struct of_device_id exynos_usi_dt_match[] = {
> {
> .compatible = "samsung,exynos850-usi",
> .data = &exynos850_usi_data,
> },
> + {
> + .compatible = "samsung,exynos8895-usi",
> + .data = &exynos8895_usi_data,
> + },
> { } /* sentinel */
> };
> MODULE_DEVICE_TABLE(of, exynos_usi_dt_match);
> @@ -105,18 +141,32 @@ static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode)
> {
> unsigned int val;
> int ret;
> + const char *name;
>
> + usi->mode = mode;
> if (mode < usi->data->min_mode || mode > usi->data->max_mode)
> return -EINVAL;
>
> - val = exynos_usi_modes[mode].val;
> + switch (usi->data->ver) {
> + case USI_VER1:
> + val = exynos_usi_v1_modes[mode].val;
> + name = exynos_usi_v1_modes[usi->mode].name;
> + break;
> + case USI_VER2:
> + val = exynos_usi_modes[mode].val;
> + name = exynos_usi_modes[usi->mode].name;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> ret = regmap_update_bits(usi->sysreg, usi->sw_conf,
> usi->data->sw_conf_mask, val);
> +
No, why? Drop.
Best regards,
Krzysztof
