| Message ID | 20241214220419.723100-2-ivo.ivanov.ivanov1@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: exynos5: Add support for Exynos8895 SoC | expand |
On 14/12/2024 23:04, Ivaylo Ivanov wrote: > Add samsung,exynos8895-hsi2c dedicated compatible for representing > I2C of Exynos8895 SoC. Since there are I2C buses that aren't implemented > as a part of USIv1 blocks, they only require a single clock. > > Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > --- > .../devicetree/bindings/i2c/i2c-exynos5.yaml | 26 ++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml > index cc8bba553..b029be88e 100644 > --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml > +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml > @@ -25,6 +25,7 @@ properties: > - samsung,exynos5250-hsi2c # Exynos5250 and Exynos5420 > - samsung,exynos5260-hsi2c # Exynos5260 > - samsung,exynos7-hsi2c # Exynos7 > + - samsung,exynos8895-hsi2c > - samsung,exynosautov9-hsi2c > - items: > - enum: > @@ -94,9 +95,28 @@ allOf: > - clock-names > > else: > - properties: > - clocks: > - maxItems: 1 > + if: > + properties: > + compatible: > + contains: > + enum: > + - samsung,exynos8895-hsi2c > + > + then: > + properties: > + clocks: Missing minItems > + maxItems: 2 > + > + clock-names: Ditto > + maxItems: 2 > + > + required: > + - clock-names I don't understand why do you need second, same branch in if, basically duplicating previous. But regardless of that, no nesting of ifs. Define clocks for all variants explicitly. Best regards, Krzysztof
On 12/16/24 10:44, Krzysztof Kozlowski wrote: > On 14/12/2024 23:04, Ivaylo Ivanov wrote: >> Add samsung,exynos8895-hsi2c dedicated compatible for representing >> I2C of Exynos8895 SoC. Since there are I2C buses that aren't implemented >> as a part of USIv1 blocks, they only require a single clock. >> >> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> >> --- >> .../devicetree/bindings/i2c/i2c-exynos5.yaml | 26 ++++++++++++++++--- >> 1 file changed, 23 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml >> index cc8bba553..b029be88e 100644 >> --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml >> +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml >> @@ -25,6 +25,7 @@ properties: >> - samsung,exynos5250-hsi2c # Exynos5250 and Exynos5420 >> - samsung,exynos5260-hsi2c # Exynos5260 >> - samsung,exynos7-hsi2c # Exynos7 >> + - samsung,exynos8895-hsi2c >> - samsung,exynosautov9-hsi2c >> - items: >> - enum: >> @@ -94,9 +95,28 @@ allOf: >> - clock-names >> >> else: >> - properties: >> - clocks: >> - maxItems: 1 >> + if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - samsung,exynos8895-hsi2c >> + >> + then: >> + properties: >> + clocks: > Missing minItems > >> + maxItems: 2 >> + >> + clock-names: > Ditto > >> + maxItems: 2 >> + >> + required: >> + - clock-names > I don't understand why do you need second, same branch in if, basically Because, as I stated in the commit message, we have HSI2C controllers both implemented in USIv1 blocks and outside. These that are a part of USIv1 need 2 clocks, and those that aren't have only one. So it's not a duplicate for the previous - autov9 sets a minitems of 2 and the others have a maxitems of 1. Best regards, Ivo > duplicating previous. But regardless of that, no nesting of ifs. Define > clocks for all variants explicitly. > > Best regards, > Krzysztof
On 17/12/2024 10:08, Ivaylo Ivanov wrote: >>>>> - items: >>>>> - enum: >>>>> @@ -94,9 +95,28 @@ allOf: >>>>> - clock-names >>>>> >>>>> else: >>>>> - properties: >>>>> - clocks: >>>>> - maxItems: 1 >>>>> + if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - samsung,exynos8895-hsi2c >>>>> + >>>>> + then: >>>>> + properties: >>>>> + clocks: >>>> Missing minItems >>>> >>>>> + maxItems: 2 >>>>> + >>>>> + clock-names: >>>> Ditto >>>> >>>>> + maxItems: 2 >>>>> + >>>>> + required: >>>>> + - clock-names >>>> I don't understand why do you need second, same branch in if, basically >>> Because, as I stated in the commit message, we have HSI2C controllers >>> both implemented in USIv1 blocks and outside. These that are a part of >> On Exynos8895? Where? With the same compatible? > > hsi2c_0 which has a clock from BUSC and hsi2c_1 to hsi2c_4 which use clocks > from PERIC1 (CLK_GOUT_PERIC1_HSI2C_CAM{0,1,2,3}_IPCLK). Why would > they need a different compatible though? It's functionally the same i2c design > as the one implemented in USIv1 blocks. If one block is part of USI and other not, they might not be the same I2C blocks, even if interface is similar. If they were the same or even functionally the same, they would have the same clock inputs. However user manual also suggests that there is only one clock, not two (for both cases), so they could be functionally equivalent but then number of clocks looks incorrect. Best regards, Krzysztof
On 17/12/2024 10:31, Ivaylo Ivanov wrote: > On 12/17/24 11:26, Krzysztof Kozlowski wrote: >> On 17/12/2024 10:08, Ivaylo Ivanov wrote: >>>>>>> - items: >>>>>>> - enum: >>>>>>> @@ -94,9 +95,28 @@ allOf: >>>>>>> - clock-names >>>>>>> >>>>>>> else: >>>>>>> - properties: >>>>>>> - clocks: >>>>>>> - maxItems: 1 >>>>>>> + if: >>>>>>> + properties: >>>>>>> + compatible: >>>>>>> + contains: >>>>>>> + enum: >>>>>>> + - samsung,exynos8895-hsi2c >>>>>>> + >>>>>>> + then: >>>>>>> + properties: >>>>>>> + clocks: >>>>>> Missing minItems >>>>>> >>>>>>> + maxItems: 2 >>>>>>> + >>>>>>> + clock-names: >>>>>> Ditto >>>>>> >>>>>>> + maxItems: 2 >>>>>>> + >>>>>>> + required: >>>>>>> + - clock-names >>>>>> I don't understand why do you need second, same branch in if, basically >>>>> Because, as I stated in the commit message, we have HSI2C controllers >>>>> both implemented in USIv1 blocks and outside. These that are a part of >>>> On Exynos8895? Where? With the same compatible? >>> hsi2c_0 which has a clock from BUSC and hsi2c_1 to hsi2c_4 which use clocks >>> from PERIC1 (CLK_GOUT_PERIC1_HSI2C_CAM{0,1,2,3}_IPCLK). Why would >>> they need a different compatible though? It's functionally the same i2c design >>> as the one implemented in USIv1 blocks. >> If one block is part of USI and other not, they might not be the same >> I2C blocks, even if interface is similar. If they were the same or even >> functionally the same, they would have the same clock inputs. However > > I see, so in such case I should make samsung,exynos8895-hsi2c-nonusi or > something like that? > >> user manual also suggests that there is only one clock, not two (for >> both cases), so they could be functionally equivalent but then number of >> clocks looks incorrect. > > That'd be weird. Both according to downstream and upstream clk driver, > for the USI-implemented i2cs we have a pclk and an sclk_usi. Something is not precise here, as usually with Samsung clock topology. First, the non-USI instances have the IPCLK as well, e.g. things like PERIC1_UID_HSI2C_CAM1_IPCLKPORT_iPCLK USI have BLK_PERIC0_UID_USI03_IPCLKPORT_i_SCLK_USI, but that's USI clock, not HSI2C in USI. Datasheet mentions this is UART and SPI special clock, but not I2C. The PCLK is used for HSI2C iPCLK. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml index cc8bba553..b029be88e 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.yaml @@ -25,6 +25,7 @@ properties: - samsung,exynos5250-hsi2c # Exynos5250 and Exynos5420 - samsung,exynos5260-hsi2c # Exynos5260 - samsung,exynos7-hsi2c # Exynos7 + - samsung,exynos8895-hsi2c - samsung,exynosautov9-hsi2c - items: - enum: @@ -94,9 +95,28 @@ allOf: - clock-names else: - properties: - clocks: - maxItems: 1 + if: + properties: + compatible: + contains: + enum: + - samsung,exynos8895-hsi2c + + then: + properties: + clocks: + maxItems: 2 + + clock-names: + maxItems: 2 + + required: + - clock-names + + else: + properties: + clocks: + maxItems: 1 unevaluatedProperties: false
Add samsung,exynos8895-hsi2c dedicated compatible for representing I2C of Exynos8895 SoC. Since there are I2C buses that aren't implemented as a part of USIv1 blocks, they only require a single clock. Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> --- .../devicetree/bindings/i2c/i2c-exynos5.yaml | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)