[PATCH 11/16] mmc: renesas-sdhi: Initialize module on RZ/G2L

[Marek Vasut]

On 10/5/23 18:35, Paul Barker wrote:
> On 03/10/2023 14:25, Marek Vasut wrote:
>> On 9/20/23 14:42, Paul Barker wrote:
>>> On the Renesas RZ/G2L SoC family, we must ensure that the required clock
>>> signals are enabled and the reset signal is de-asserted before we try to
>>> communicate with the SDHI module.
>>>
>>> Signed-off-by: Paul Barker <paul.barker.ct at bp.renesas.com>
>>> Reviewed-by: Biju Das <biju.das.jz at bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>
>>> ---
>>>    drivers/mmc/renesas-sdhi.c | 61 ++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
>>> index 8e716f74491f..170c5dcc2ebe 100644
>>> --- a/drivers/mmc/renesas-sdhi.c
>>> +++ b/drivers/mmc/renesas-sdhi.c
>>> @@ -20,6 +20,7 @@
>>>    #include <linux/io.h>
>>>    #include <linux/sizes.h>
>>>    #include <power/regulator.h>
>>> +#include <reset.h>
>>>    #include <asm/unaligned.h>
>>>    #include "tmio-common.h"
>>>    
>>> @@ -964,6 +965,8 @@ static int renesas_sdhi_probe(struct udevice *dev)
>>>    	u32 quirks = dev_get_driver_data(dev);
>>>    	struct fdt_resource reg_res;
>>>    	DECLARE_GLOBAL_DATA_PTR;
>>> +	struct clk imclk2, aclk;
>>> +	struct reset_ctl rst;
>>>    	int ret;
>>>    
>>>    	priv->clk_get_rate = renesas_sdhi_clk_get_rate;
>>> @@ -1012,6 +1015,49 @@ static int renesas_sdhi_probe(struct udevice *dev)
>>>    		goto err_clkh;
>>>    	}
>>>    
>>> +	if (IS_ENABLED(CONFIG_RZG2L)) {
>>> +		/*
>>> +		 * On members of the RZ/G2L SoC family, we need to enable
>>> +		 * additional chip detect and bus clocks, then release the SDHI
>>> +		 * module from reset.
>>> +		 */
>>
>> This could use a separate function, and then, use bulk clock API via
>> clk_get_bulk() and co .
> 
> There are 4 clocks defined in the dtb: "core", "clkh", "cd", "aclk".
> 
> During probe, we want to enable "core", "cd" (aka "imclk2" in the
> datasheet) and "aclk" but not "clkh". The driver enables "clkh" later as
> needed for high speed modes. So I don't think we want to bulk enable
> everything here.

Aha, OK

> By "separate function", are you suggesting an entire separate probe
> function for RZ/G2L SDHI? Or just moving what's inside the
> if(IS_ENABLED(CONFIG_RZG2L)) into a new function which we can call from
> renesas_sdhi_probe()?

The later, to avoid this huge ifdef within the probe

if (IS_ENABLED(CONFIG_RZG2L))
    rzg2l_do_whatever_extra_stuff_is_needed();

Thanks !