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@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>> 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
