On 12/17/24 11:43, Krzysztof Kozlowski wrote: > 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. That's weird. Don't we need the clock enabled in order for the USIv1's HSI2C to work? Best regards, Ivo > The PCLK is used for HSI2C iPCLK. > > > Best regards, > Krzysztof