On 29/09/2022 17:18, Dinh Nguyen wrote:
>
>
> On 9/29/22 09:38, Krzysztof Kozlowski wrote:
>> On 29/09/2022 16:20, Dinh Nguyen wrote:
>>>>
>>>> So this change will not be backwards compatible with existing DTBs. I
>>>> noticed that patch2 updates the DTS files for the arm64 platforms, but
>>>> there seems to be some arm32 platforms too. Isn't this going to be a
>>>> problem?
>>>>
>>>
>>> The arm32 platforms makes the clk-phase adjustment through the clock
>>> driver. There was a discussion when I originally submitted the support
>>> for the arm32 platforms, and we landed on going through the clock driver
>>> instead of using the MMC driver. The updates to the arm32 platforms can
>>> be done after this patch series.
>>
>> How the update "can be done after"? Didn't you break all boards in- and
>> out-of-tree?
>>
>
> I don't think so! At least, I don't see how, for the arm32 boards, here
> are the dts entry for setting the clock-phase:
>
> sdmmc_clk: sdmmc_clk {
> #clock-cells = <0>;
> compatible = "altr,socfpga-gate-clk";
> clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>,<&per_nand_mmc_clk>;
> clk-gate = <0xa0 8>;
> clk-phase = <0 135>; <-----
It's different node...
> };
>
> sdmmc_clk_divided: sdmmc_clk_divided {
> #clock-cells = <0>;
> compatible = "altr,socfpga-gate-clk";
> clocks = <&sdmmc_clk>;
> clk-gate = <0xa0 8>;
> fixed-divider = <4>;
> };
>
> ...
> mmc: dwmmc0@ff704000 {
> compatible = "altr,socfpga-dw-mshc";
> reg = <0xff704000 0x1000>;
> interrupts = <0 139 4>;
> fifo-depth = <0x400>;
> #address-cells = <1>;
> #size-cells = <0>;
> clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>;
> clock-names = "biu", "ciu";
> resets = <&rst SDMMC_RESET>;
> status = "disabled";
And this one does not have clk-phase-sd-hs
> };
>
>
> So the setting for the clk-phase is done in the clock driver,
> (drivers/clk/socfpga/clk-gate.c). This has been done many years now,
> before the clk-phase-hs-sd concept was added to the sdmmc driver.
Yes and the driver now requires clk-phase-sd-hs or altr,sysmgr-syscon
which is not present in DTS.
>
> When I originally submitted the patches for the ARM64 clock driver
> support, I forgot to add the clk-phase support for the SD controller.
> Now that I realized we needed it, the concept to set the clk-phase is in
> the SD driver, thus I'm just adding the support for arm64.
>
> The arm32 support does not change in any way, so I don't see how it will
> break it.
Isn't your driver returning ERRNO for all existing DTS (so without patch
#2) and for all out of tree DTS?
>
> I can update the arm32 support with the same function in patch3 after
> this series. Because updating the arm32 will require me to remove the
> support in the clock driver, thus, I want to break it out.
Best regards,
Krzysztof
