Message ID | 20240119111132.1290455-1-tudor.ambarus@linaro.org |
---|---|
Headers | show |
Series | GS101 Oriole: CMU_PERIC0 support and USI updates | expand |
On 19/01/2024 12:11, Tudor Ambarus wrote: > CMU_PERIC0 is the clock management unit used for the peric0 block which > is used for USI and I3C. Add support for all cmu_peric0 clocks but > CLK_GOUT_PERIC0_IP (not enough info in the datasheet). > > Few clocks are marked as critical because when either of them is > disabled, the system hangs even if their clock parents are enabled. > > Reviewed-by: Peter Griffin <peter.griffin@linaro.org> > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > drivers/clk/samsung/clk-gs101.c | 583 ++++++++++++++++++++++++++++++++ > 1 file changed, 583 insertions(+) > This does not apply. Please rebase on my samsung for-next or next linux-next and resend. Best regards, Krzysztof
On Fri, 19 Jan 2024 11:11:28 +0000, Tudor Ambarus wrote: > Remove the reg-io-width property in order to comply with the bindings. > > The entire bus (PERIC) on which the GS101 serial resides only allows > 32-bit register accesses. The reg-io-width dt property is disallowed > for the "google,gs101-uart" compatible and instead the iotype is > inferred from the compatible. > > [...] Applied, thanks! [4/8] arm64: dts: exynos: gs101: remove reg-io-width from serial https://git.kernel.org/krzk/linux/c/daff9d192892ea583284eb116a07e8e0086f0e76 Best regards,
On Fri, 19 Jan 2024 11:11:29 +0000, Tudor Ambarus wrote: > Enable the cmu-peric0 clock controller. It feeds USI and I3c. > > Applied, thanks! [5/8] arm64: dts: exynos: gs101: enable cmu-peric0 clock controller https://git.kernel.org/krzk/linux/c/8a670bb84cdcf1397fc4a3bc295c0008f49bed91 Best regards,
On Fri, 19 Jan 2024 11:11:30 +0000, Tudor Ambarus wrote: > Get rid of the dummy clock and start using the cmu_peric0 clocks > for the usi_uart and serial_0 nodes. > > Tested the serial at 115200, 1000000 and 3000000 baudrates, > everthing went fine. > > > [...] Applied, thanks! [6/8] arm64: dts: exynos: gs101: update USI UART to use peric0 clocks https://git.kernel.org/krzk/linux/c/e58513b2ff78c70805d3bd7d96bfd76576d4c9e3 Best regards,
On Fri, 19 Jan 2024 11:11:31 +0000, Tudor Ambarus wrote: > USI8 I2C is used to communicate with an eeprom found on the battery > connector. Define USI8 in I2C configuration. > > USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 > doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the > selection of the protocol is intentionally left for the board dts file. > > [...] Applied, thanks! [7/8] arm64: dts: exynos: gs101: define USI8 with I2C configuration https://git.kernel.org/krzk/linux/c/9ca7055a35a7b2b373ead6f3a67ee8b5e0e6e468 Best regards,
On Fri, 19 Jan 2024 11:11:32 +0000, Tudor Ambarus wrote: > Enable the eeprom found on the battery connector. > > The selection of the USI protocol is done in the board dts file because > the USI CONFIG register comes with a 0x0 reset value, meaning that USI8 > does not have a default protocol (I2C, SPI, UART) at reset. > > > [...] Applied, thanks! [8/8] arm64: dts: exynos: gs101: enable eeprom on gs101-oriole https://git.kernel.org/krzk/linux/c/6a06935ec8263be083ac897a842f06d66b138ffb Best regards,
On 1/22/24 11:13, Krzysztof Kozlowski wrote: > On 19/01/2024 12:11, Tudor Ambarus wrote: >> CMU_PERIC0 is the clock management unit used for the peric0 block which >> is used for USI and I3C. Add support for all cmu_peric0 clocks but >> CLK_GOUT_PERIC0_IP (not enough info in the datasheet). >> >> Few clocks are marked as critical because when either of them is >> disabled, the system hangs even if their clock parents are enabled. >> >> Reviewed-by: Peter Griffin <peter.griffin@linaro.org> >> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >> --- >> drivers/clk/samsung/clk-gs101.c | 583 ++++++++++++++++++++++++++++++++ >> 1 file changed, 583 insertions(+) >> > > This does not apply. Please rebase on my samsung for-next or next > linux-next and resend. > Oh, yes. I rebased, did a boot test and then sent v5 at: https://lore.kernel.org/linux-arm-kernel/20240122114113.2582612-1-tudor.ambarus@linaro.org/T/#u Thanks! ta
On 19/01/2024 12:11, Tudor Ambarus wrote: > USI8 I2C is used to communicate with an eeprom found on the battery > connector. Define USI8 in I2C configuration. > > USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 > doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the > selection of the protocol is intentionally left for the board dts file. ... and dropped, because this patch does not build: https://krzk.eu/#/builders/29/builds/3869 and I missed weird dependency mentioned in cover letter: "This patch set shall be queued after the cmu_misc clock name fixes from:" Sorry, this cannot work like that. DTS for new features cannot build depend on driver changes. Best regards, Krzysztof
On 19/01/2024 12:11, Tudor Ambarus wrote: > Get rid of the dummy clock and start using the cmu_peric0 clocks > for the usi_uart and serial_0 nodes. > > Tested the serial at 115200, 1000000 and 3000000 baudrates, > everthing went fine. > > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> This also fails to build (and also cannot depend on driver changes because it is a new DTS). Dropped. Best regards, Krzysztof
On 1/23/24 07:52, Krzysztof Kozlowski wrote: > On 19/01/2024 12:11, Tudor Ambarus wrote: >> USI8 I2C is used to communicate with an eeprom found on the battery >> connector. Define USI8 in I2C configuration. >> >> USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 >> doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the >> selection of the protocol is intentionally left for the board dts file. > > ... and dropped, because this patch does not build: > https://krzk.eu/#/builders/29/builds/3869 > and I missed weird dependency mentioned in cover letter: > > "This patch set shall be queued after the cmu_misc clock name fixes from:" > > Sorry, this cannot work like that. DTS for new features cannot build > depend on driver changes. No worries. What shall I do so that you re-consider the dropped patches? I'm not yet familiar with your release management, but I guess that if you submit your "fixes-clk" branch for integration into v6.8-rc2, and then merge v6.8-rc2 into your "next/dt64", you'll then be able to queue the dropped patches as well. Thanks, ta
On 23/01/2024 09:34, Tudor Ambarus wrote: > > > On 1/23/24 07:52, Krzysztof Kozlowski wrote: >> On 19/01/2024 12:11, Tudor Ambarus wrote: >>> USI8 I2C is used to communicate with an eeprom found on the battery >>> connector. Define USI8 in I2C configuration. >>> >>> USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 >>> doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the >>> selection of the protocol is intentionally left for the board dts file. >> >> ... and dropped, because this patch does not build: >> https://krzk.eu/#/builders/29/builds/3869 >> and I missed weird dependency mentioned in cover letter: >> >> "This patch set shall be queued after the cmu_misc clock name fixes from:" >> >> Sorry, this cannot work like that. DTS for new features cannot build >> depend on driver changes. > > No worries. What shall I do so that you re-consider the dropped patches? > I'm not yet familiar with your release management, but I guess that if > you submit your "fixes-clk" branch for integration into v6.8-rc2, and > then merge v6.8-rc2 into your "next/dt64", you'll then be able to queue > the dropped patches as well. It is nothing specific to my release management but years old rule: DTS branch cannot contain driver commits. It is nothing new, discussed on mailing lists for various SoC architectures many times. However I don't fully understand why that dependency - except patch hunk context - exists. You shouldn't have such dependency. Best regards, Krzysztof
On 1/23/24 08:39, Krzysztof Kozlowski wrote: > On 23/01/2024 09:34, Tudor Ambarus wrote: >> >> >> On 1/23/24 07:52, Krzysztof Kozlowski wrote: >>> On 19/01/2024 12:11, Tudor Ambarus wrote: >>>> USI8 I2C is used to communicate with an eeprom found on the battery >>>> connector. Define USI8 in I2C configuration. >>>> >>>> USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 >>>> doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the >>>> selection of the protocol is intentionally left for the board dts file. >>> >>> ... and dropped, because this patch does not build: >>> https://krzk.eu/#/builders/29/builds/3869 >>> and I missed weird dependency mentioned in cover letter: >>> >>> "This patch set shall be queued after the cmu_misc clock name fixes from:" >>> >>> Sorry, this cannot work like that. DTS for new features cannot build >>> depend on driver changes. >> >> No worries. What shall I do so that you re-consider the dropped patches? >> I'm not yet familiar with your release management, but I guess that if >> you submit your "fixes-clk" branch for integration into v6.8-rc2, and >> then merge v6.8-rc2 into your "next/dt64", you'll then be able to queue >> the dropped patches as well. > > It is nothing specific to my release management but years old rule: DTS > branch cannot contain driver commits. It is nothing new, discussed on > mailing lists for various SoC architectures many times. Okay, thanks for the explanation. > > However I don't fully understand why that dependency - except patch hunk > context - exists. You shouldn't have such dependency. > Let me try offline, I'll get back to you.
On 1/23/24 08:44, Tudor Ambarus wrote: >> However I don't fully understand why that dependency - except patch hunk >> context - exists. You shouldn't have such dependency. >> > Let me try offline, I'll get back to you. The dropped patches depend on the dt-bindings patch that you queued through the "next/drivers" branch: b393a6c5e656 dt-bindings: clock: google,gs101-clock: add PERIC0 clock management unit We need the peric0 bindings that are referenced in device tree, that's why the next/dt64 branch failed to build. Please let me know if there's something on my side that I have to do (now or in the future). Thanks, ta
On 23/01/2024 09:57, Tudor Ambarus wrote: > > > On 1/23/24 08:44, Tudor Ambarus wrote: >>> However I don't fully understand why that dependency - except patch hunk >>> context - exists. You shouldn't have such dependency. >>> >> Let me try offline, I'll get back to you. > > The dropped patches depend on the dt-bindings patch that you queued > through the "next/drivers" branch: > > b393a6c5e656 dt-bindings: clock: google,gs101-clock: add PERIC0 clock > management unit > > We need the peric0 bindings that are referenced in device tree, that's > why the next/dt64 branch failed to build. > > Please let me know if there's something on my side that I have to do > (now or in the future). It is useful to mention this in cover letter, so I will know how to apply the patches. I understand therefore the dependency mention in the cover letter is not accurate, so I can ignore that aspect. Best regards, Krzysztof
On 1/23/24 08:59, Krzysztof Kozlowski wrote: > On 23/01/2024 09:57, Tudor Ambarus wrote: >> >> >> On 1/23/24 08:44, Tudor Ambarus wrote: >>>> However I don't fully understand why that dependency - except patch hunk >>>> context - exists. You shouldn't have such dependency. >>>> >>> Let me try offline, I'll get back to you. >> >> The dropped patches depend on the dt-bindings patch that you queued >> through the "next/drivers" branch: >> >> b393a6c5e656 dt-bindings: clock: google,gs101-clock: add PERIC0 clock >> management unit >> >> We need the peric0 bindings that are referenced in device tree, that's >> why the next/dt64 branch failed to build. >> >> Please let me know if there's something on my side that I have to do >> (now or in the future). > > It is useful to mention this in cover letter, so I will know how to > apply the patches. I understand therefore the dependency mention in the Thanks for the patience, I learn along the way. > cover letter is not accurate, so I can ignore that aspect. > Yes, that's right. The dependency on name fixes is just as a patch hunk context, no functional or build dependencies. I now know the process, and I'll be more verbose next time. Cheers, ta
On Fri, 19 Jan 2024 11:11:30 +0000, Tudor Ambarus wrote: > Get rid of the dummy clock and start using the cmu_peric0 clocks > for the usi_uart and serial_0 nodes. > > Tested the serial at 115200, 1000000 and 3000000 baudrates, > everthing went fine. > > > [...] Applied, thanks! [6/8] arm64: dts: exynos: gs101: update USI UART to use peric0 clocks https://git.kernel.org/krzk/linux/c/3dfbd155b2e397f677f18fd3eccb8691443fb280 Best regards,
On Fri, 19 Jan 2024 11:11:31 +0000, Tudor Ambarus wrote: > USI8 I2C is used to communicate with an eeprom found on the battery > connector. Define USI8 in I2C configuration. > > USI8 CONFIG register comes with a 0x0 reset value, meaning that USI8 > doesn't have a default protocol (I2C, SPI, UART) at reset. Thus the > selection of the protocol is intentionally left for the board dts file. > > [...] Applied, thanks! [7/8] arm64: dts: exynos: gs101: define USI8 with I2C configuration https://git.kernel.org/krzk/linux/c/f3635d5ff6105e6e0450b2e7f7bb0055f0fea305 Best regards,
On Fri, 19 Jan 2024 11:11:32 +0000, Tudor Ambarus wrote: > Enable the eeprom found on the battery connector. > > The selection of the USI protocol is done in the board dts file because > the USI CONFIG register comes with a 0x0 reset value, meaning that USI8 > does not have a default protocol (I2C, SPI, UART) at reset. > > > [...] Applied, thanks! [8/8] arm64: dts: exynos: gs101: enable eeprom on gs101-oriole https://git.kernel.org/krzk/linux/c/c68efa676ca8febfd36aab50fe747c072c224d52 Best regards,