[1/2] arm64: dts: socfpga: Add clk-phase-sd-hs property to the sdmmc node

The sdmmc controller's CIU(Card Interface Unit) clock's phase can be
adjusted through the register in the system manager. Add the binding
"altr,sysmgr-syscon" to the SDMMC node for the driver to access the
system manager. Add the "clk-phase-sd-hs" property in the SDMMC node to
designate the smpsel and drvsel properties for the CIU clock.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi      | 1 +
 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts | 1 +
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi          | 1 +
 arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts     | 1 +
 arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dts        | 1 +
 5 files changed, 5 insertions(+)

On 20/09/2022 15:24, Dinh Nguyen wrote:
> 
> Hi Ulf,
> 
> Thanks for the review!
> 
> On 9/20/22 07:17, Ulf Hansson wrote:
>> On Mon, 19 Sept 2022 at 20:13, Dinh Nguyen <dinguyen@kernel.org> wrote:
>>>
>>> The clock-phase settings for the SDMMC controller in the SoCFPGA
>>> Strarix10/Agilex/N5X platforms reside in a register in the System
>>> Manager. Add a method to access that register through the syscon
>>> interface.
>>>
>>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>>> ---
>>>   drivers/mmc/host/dw_mmc-pltfm.c | 68 ++++++++++++++++++++++++++++++++-
>>>   1 file changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>>> index 9901208be797..9e3237c18a9d 100644
>>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>>> @@ -17,10 +17,15 @@
>>>   #include <linux/mmc/host.h>
>>>   #include <linux/mmc/mmc.h>
>>>   #include <linux/of.h>
>>> +#include <linux/mfd/altera-sysmgr.h>
>>> +#include <linux/regmap.h>
>>>
>>>   #include "dw_mmc.h"
>>>   #include "dw_mmc-pltfm.h"
>>>
>>> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
>>> +       ((((smplsel) & 0x7) << 4) | (((drvsel) & 0x7) << 0))
>>> +
>>>   int dw_mci_pltfm_register(struct platform_device *pdev,
>>>                            const struct dw_mci_drv_data *drv_data)
>>>   {
>>> @@ -62,9 +67,70 @@ const struct dev_pm_ops dw_mci_pltfm_pmops = {
>>>   };
>>>   EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
>>>
>>> +static int dw_mci_socfpga_priv_init(struct dw_mci *host)
>>> +{
>>> +       struct device_node *np = host->dev->of_node;
>>> +       struct regmap *sys_mgr_base_addr;
>>> +       u32 clk_phase[2] = {0}, reg_offset;
>>> +       int i, rc, hs_timing;
>>> +
>>> +       rc = of_property_read_variable_u32_array(np, "clk-phase-sd-hs", &clk_phase[0], 2, 0);
>>
>> This needs to be documented through updated DT bindings.
> 
> Ok, but it looks like clk-phase-sd-hs is already documented in 
> Documentation/devicetree/bindings/mmc/mmc-controller.yaml

Not in next-20220919.

> 
> Should I create a specific documentation just for
> "altr,socfpga-dw-mshc" and document "clk-phase-sd-hs"?

All properties must be documented.

> 
>>
>>> +       if (rc) {
>>> +               sys_mgr_base_addr =
>>> +                       altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
>>
>> DT bindings?
> 
> "altr,sysmgr-syscon" has already been documented in
> Documentation/devicetree/bindings/net/socfpga-dwmac.txt

This is not documentation of nodes you are changing here and in patch 1.

You linked altr,socfpga-stmmac and here you have altr,socfpga-dw-mshc...

Best regards,
Krzysztof

On 19/09/2022 20:13, Dinh Nguyen wrote:
> The sdmmc controller's CIU(Card Interface Unit) clock's phase can be
> adjusted through the register in the system manager. Add the binding
> "altr,sysmgr-syscon" to the SDMMC node for the driver to access the
> system manager. Add the "clk-phase-sd-hs" property in the SDMMC node to
> designate the smpsel and drvsel properties for the CIU clock.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi      | 1 +
>  arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts | 1 +
>  arch/arm64/boot/dts/intel/socfpga_agilex.dtsi          | 1 +
>  arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts     | 1 +
>  arch/arm64/boot/dts/intel/socfpga_n5x_socdk.dts        | 1 +
>  5 files changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index 14c220d87807..a5d08920ac81 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -309,6 +309,7 @@ mmc: mmc@ff808000 {
>  				 <&clkmgr STRATIX10_SDMMC_CLK>;
>  			clock-names = "biu", "ciu";
>  			iommus = <&smmu 5>;
> +			altr,sysmgr-syscon = <&sysmgr 0x28 0>;

Missing bindings change.


Best regards,
Krzysztof