| Message ID | 20250105160346.418829-1-ivo.ivanov.ivanov1@gmail.com |
|---|---|
| Headers | show |
| Series | soc: samsung: usi: implement support for USIv1 | expand |
On Sun, Jan 05, 2025 at 06:03:43PM +0200, Ivaylo Ivanov wrote: > diff --git a/include/dt-bindings/soc/samsung,exynos-usi.h b/include/dt-bindings/soc/samsung,exynos-usi.h > index a01af169d..b7c1406f3 100644 > --- a/include/dt-bindings/soc/samsung,exynos-usi.h > +++ b/include/dt-bindings/soc/samsung,exynos-usi.h > @@ -9,9 +9,9 @@ > #ifndef __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H > #define __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H > > -#define USI_V2_NONE 0 > -#define USI_V2_UART 1 > -#define USI_V2_SPI 2 > -#define USI_V2_I2C 3 > +#define USI_MODE_NONE 0 > +#define USI_MODE_UART 1 > +#define USI_MODE_SPI 2 > +#define USI_MODE_I2C 3 This breaks ABI and does not build. You still need also: #define USI_V2_NONE USI_MODE_NONE and same for the rest. Best regards, Krzysztof
On Sun, Jan 05, 2025 at 06:03:45PM +0200, Ivaylo Ivanov wrote: > @@ -169,9 +207,12 @@ static int exynos_usi_configure(struct exynos_usi *usi) > return ret; > > if (usi->data->ver == USI_VER2) > - return exynos_usi_enable(usi); > + ret = exynos_usi_enable(usi); > + else > + ret = clk_bulk_prepare_enable(usi->data->num_clks, > + usi->clks); You need now exynos_usi_remove() callback and also error path after populate at end of probe. Best regards, Krzysztof
On 1/6/25 08:24, Krzysztof Kozlowski wrote: > On Sun, Jan 05, 2025 at 06:03:43PM +0200, Ivaylo Ivanov wrote: >> diff --git a/include/dt-bindings/soc/samsung,exynos-usi.h b/include/dt-bindings/soc/samsung,exynos-usi.h >> index a01af169d..b7c1406f3 100644 >> --- a/include/dt-bindings/soc/samsung,exynos-usi.h >> +++ b/include/dt-bindings/soc/samsung,exynos-usi.h >> @@ -9,9 +9,9 @@ >> #ifndef __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H >> #define __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H >> >> -#define USI_V2_NONE 0 >> -#define USI_V2_UART 1 >> -#define USI_V2_SPI 2 >> -#define USI_V2_I2C 3 >> +#define USI_MODE_NONE 0 >> +#define USI_MODE_UART 1 >> +#define USI_MODE_SPI 2 >> +#define USI_MODE_I2C 3 > This breaks ABI and does not build. You still need also: > #define USI_V2_NONE USI_MODE_NONE > and same for the rest. Alright, sounds good to me. That way I shouldn't s/USI_V2/USI_MODE/g for the other SoC device trees, right? Should I also mention with a comment that the V2 constants are deprecated? Best regards, Ivaylo > > Best regards, > Krzysztof >
On 06/01/2025 08:09, Ivaylo Ivanov wrote: >>> >>> -#define USI_V2_NONE 0 >>> -#define USI_V2_UART 1 >>> -#define USI_V2_SPI 2 >>> -#define USI_V2_I2C 3 >>> +#define USI_MODE_NONE 0 >>> +#define USI_MODE_UART 1 >>> +#define USI_MODE_SPI 2 >>> +#define USI_MODE_I2C 3 >> This breaks ABI and does not build. You still need also: >> #define USI_V2_NONE USI_MODE_NONE >> and same for the rest. > > Alright, sounds good to me. That way I shouldn't s/USI_V2/USI_MODE/g > for the other SoC device trees, right? Should I also mention with a You can change them as well, because USI_MODE_XXX will be preferred from now on. The DTS commit will just wait one cycle till bindings get accepted. > comment that the V2 constants are deprecated? Yes, this would be good. Thanks for working on this. Best regards, Krzysztof
Hiya, On 1/5/25 4:03 PM, Ivaylo Ivanov wrote: > +#define USI_MODE_NONE 0 > +#define USI_MODE_UART 1 > +#define USI_MODE_SPI 2 > +#define USI_MODE_I2C 3 USI_CONFIG register refers to the protocol selection with USI_I2C, USI_SPI, USI_UART. How about getting rid of the MODE from the name? Cheers, ta
On 1/6/25 09:36, Tudor Ambarus wrote: > Hiya, > > On 1/5/25 4:03 PM, Ivaylo Ivanov wrote: >> +#define USI_MODE_NONE 0 >> +#define USI_MODE_UART 1 >> +#define USI_MODE_SPI 2 >> +#define USI_MODE_I2C 3 > USI_CONFIG register refers to the protocol selection with USI_I2C, > USI_SPI, USI_UART. How about getting rid of the MODE from the name? I thought about that too but I believe that mentioning that these constants are for mode selection in their name is generally a good practice. Let me know if dropping _MODE is really needed. Best regards, Ivaylo > > Cheers, > ta
On 06/01/2025 08:41, Ivaylo Ivanov wrote: > On 1/6/25 09:36, Tudor Ambarus wrote: >> Hiya, >> >> On 1/5/25 4:03 PM, Ivaylo Ivanov wrote: >>> +#define USI_MODE_NONE 0 >>> +#define USI_MODE_UART 1 >>> +#define USI_MODE_SPI 2 >>> +#define USI_MODE_I2C 3 >> USI_CONFIG register refers to the protocol selection with USI_I2C, >> USI_SPI, USI_UART. How about getting rid of the MODE from the name? > > I thought about that too but I believe that mentioning that these constants > are for mode selection in their name is generally a good practice. Let me know > if dropping _MODE is really needed. I am fine with both, MODE feels a bit more descriptive indicating how the USI should be configured. Best regards, Krzysztof
On 1/6/25 8:50 AM, Krzysztof Kozlowski wrote: > On 06/01/2025 08:41, Ivaylo Ivanov wrote: >> On 1/6/25 09:36, Tudor Ambarus wrote: >>> Hiya, >>> >>> On 1/5/25 4:03 PM, Ivaylo Ivanov wrote: >>>> +#define USI_MODE_NONE 0 >>>> +#define USI_MODE_UART 1 >>>> +#define USI_MODE_SPI 2 >>>> +#define USI_MODE_I2C 3 >>> USI_CONFIG register refers to the protocol selection with USI_I2C, >>> USI_SPI, USI_UART. How about getting rid of the MODE from the name? >> >> I thought about that too but I believe that mentioning that these constants >> are for mode selection in their name is generally a good practice. Let me know >> if dropping _MODE is really needed. no strong requirement. > I am fine with both, MODE feels a bit more descriptive indicating how > the USI should be configured. Fine by me to keep MODE in the name. Cheers, ta
Hey folks, This patch series adds support for USIv1 in the existing exynos-usi driver, as well as dedicated sysreg compatibles for exynos8895. The USIv1 IP-core is found on some ARM64 Exynos SoCs (like Exynos8895). It provides selectable serial protocols (one of: HSI2C0, HSI2C1, HSI2C0_1, SPI, UART, UART_HSI2C1). It's a bit different from USIv2 as it doesn't have any known MMIO register map and the serial protocols that it implements share the same register base, hence why the way of modelling it in device trees will be with ranges, like so: usi1: usi@10460000 { compatible = "samsung,exynos8895-usi"; ranges = <0x0 0x10460000 0x11000>; clocks = <1>, <2>; clock-names = "pclk", "ipclk"; #address-cells = <1>; #size-cells = <1>; samsung,sysreg = <&syscon_peric0 0x1004>; status = "disabled"; hsi2c_5: i2c@0 { compatible = "samsung,exynos8895-hsi2c"; reg = <0x0 0x1000>; ... }; }; This patchset also assumes that [1] and [2] have been merged before it. Best regards, Ivaylo [1]: https://lore.kernel.org/all/20241222145257.31451-1-krzysztof.kozlowski@linaro.org/ [2]: https://lore.kernel.org/all/20250103082549.19419-1-krzysztof.kozlowski@linaro.org/ Changes in v3: - drop the sysreg patch as it was applied - add a patch at the beginning of the series for renaming all USI_V2 constants to USI_MODE_ and a patch at the end to rename them in dt - redo the usi binding support for 8895 to hopefully match all feedback from Krzysztof - change the description of the usiv1 and 8895 binding patch in order to account for the constants changes - change the subject and description of the usiv1 driver support patch because we're adding support for exynos8895 in the first place - make exynos_usi_modes a two dimensional array while also accounting for the merged usi mode constants Changes in v2: - add r-b from Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> - restrict the possible ids of samsung,mode with an allOf:if:then - set the properties samsung,clkreq-on and reg to false for non-usiv2 - only make use of exynos_usi_modes - make sure pclk and ipclk are enabled Ivaylo Ivanov (4): dt-bindings: soc: samsung: usi: replace USI_V2 in constants with USI_MODE dt-bindings: soc: samsung: usi: add USIv1 and samsung,exynos8895-usi soc: samsung: usi: implement support for USIv1 and exynos8895 arm64: dts: exynos: update all samsung,mode constants .../bindings/soc/samsung/exynos-usi.yaml | 101 ++++++++++++------ arch/arm64/boot/dts/exynos/exynos850.dtsi | 14 +-- arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 48 ++++----- .../arm64/boot/dts/exynos/exynosautov920.dtsi | 2 +- .../boot/dts/exynos/google/gs101-oriole.dts | 4 +- arch/arm64/boot/dts/exynos/google/gs101.dtsi | 2 +- drivers/soc/samsung/exynos-usi.c | 65 ++++++++--- include/dt-bindings/soc/samsung,exynos-usi.h | 11 +- 8 files changed, 163 insertions(+), 84 deletions(-)