linux-kernel - Re: [PATCHv4 1/3] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 29 Sep 2022 10:18:23 -0500
From:   Dinh Nguyen <dinguyen@...nel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Ulf Hansson <ulf.hansson@...aro.org>
Cc:     jh80.chung@...sung.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, linux-mmc@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCHv4 1/3] dt-bindings: mmc: synopsys-dw-mshc: document
 "altr,sysmgr-syscon"

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>;   <-----
};

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@...04000 {
	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";
	};

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.

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.

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.

Dinh