[U-Boot] [U-Boot, v3, 1/3] mmc: dwmmc: socfpga: Add reset ctrl to driver

[Ley Foon Tan]

On Thu, May 31, 2018 at 7:16 PM, Tom Rini <trini at konsulko.com> wrote:
> On Thu, May 31, 2018 at 11:02:39AM +0800, Ley Foon Tan wrote:
>> On Fri, May 25, 2018 at 7:16 PM, Tom Rini <trini at konsulko.com> wrote:
>> > On Fri, May 25, 2018 at 10:45:53AM +0800, Ley Foon Tan wrote:
>> >> On Thu, May 24, 2018 at 8:39 PM, Tom Rini <trini at konsulko.com> wrote:
>> >> > On Tue, May 08, 2018 at 11:19:24AM +0800, Ley Foon Tan wrote:
>> >> >
>> >> >> Add code to reset all reset signals as in mmc DT node. A reset property is an optional feature,
>> >> >> so only print out a warning and do not fail if a reset property is not present.
>> >> >>
>> >> >> If a reset property is discovered, then use it to deassert, thus bringing the
>> >> >> IP out of reset.
>> >> >>
>> >> >> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>> >> >> Reviewed-by: Simon Glass <sjg at chromium.org>
>> >> >> ---
>> >> >>  drivers/mmc/socfpga_dw_mmc.c |   17 +++++++++++++++++
>> >> >>  1 files changed, 17 insertions(+), 0 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
>> >> >> index fa0e449..eb7e64e 100644
>> >> >> --- a/drivers/mmc/socfpga_dw_mmc.c
>> >> >> +++ b/drivers/mmc/socfpga_dw_mmc.c
>> >> >> @@ -13,6 +13,7 @@
>> >> >>  #include <linux/libfdt.h>
>> >> >>  #include <linux/err.h>
>> >> >>  #include <malloc.h>
>> >> >> +#include <reset.h>
>> >> >>
>> >> >>  DECLARE_GLOBAL_DATA_PTR;
>> >> >>
>> >> >> @@ -33,6 +34,20 @@ struct dwmci_socfpga_priv_data {
>> >> >>       unsigned int            smplsel;
>> >> >>  };
>> >> >>
>> >> >> +static void socfpga_dwmci_reset(struct udevice *dev)
>> >> >> +{
>> >> >> +     struct reset_ctl_bulk reset_bulk;
>> >> >> +     int ret;
>> >> >> +
>> >> >> +     ret = reset_get_bulk(dev, &reset_bulk);
>> >> >> +     if (ret) {
>> >> >> +             dev_warn(dev, "Can't get reset: %d\n", ret);
>> >> >> +             return;
>> >> >> +     }
>> >> >> +
>> >> >> +     reset_deassert_bulk(&reset_bulk);
>> >> >> +}
>> >> >
>> >> > The driver doesn't depend on DM_RESET and this code hunk doesn't either
>> >> > so it fails to build on a number of platforms.  This type of comment
>> >> > applies to the whole series, and may be fixed differently in different
>> >> > cases (it might be OK to enforce DM_RESET for this driver, but not for
>> >> > the ns16550 driver).
>> >> >
>> >> > --
>> >> > Tom
>> >> >
>> >> include/reset.h has the DM_RESET wrapper, so it will not cause the
>> >> compilation error if the CONFIG_DM_RESET is disabled.
>> >>
>> >> I have tried compile the uboot with CONFIG_DM_RESET disabled,
>> >> compilation is fine.
>> >
>> > This whole series causes a good percent of the world to fail to link, so
>> > something is off about the wrappers or use of them.  See:
>> > https://travis-ci.org/trini/u-boot/jobs/382783645
>> >
>> This build is happened in SPL build, when CONF_DM_RESET is enabled,
>> but CONFIG_SPL_RESET_SUPPORT is disabled.
>> So, adding #ifdef CONFIG_DM_RESET checking in these peripherals also
>> can't resolve the compilation error.
>> Two possible fix below, or you have better suggestion.
>>
>> (1) select SPL_RESET_SUPPORT when DM_RESET is enabled.
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 33c39b7..b43cd89 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -3,6 +3,7 @@ menu "Reset Controller Support"
>>  config DM_RESET
>>         bool "Enable reset controllers using Driver Model"
>>         depends on DM && OF_CONTROL
>> +       select SPL_RESET_SUPPORT
>>         help
>>
>>
>>
>> (2) Add CONFIG_SPL_RESET_SUPPORT checking in reset.h file
>>
>> diff --git a/include/reset.h b/include/reset.h
>> index 201bafc..0ac0a47 100644
>> --- a/include/reset.h
>> +++ b/include/reset.h
>> @@ -77,7 +77,8 @@ struct reset_ctl_bulk {
>>         unsigned int count;
>>  };
>>
>> -#ifdef CONFIG_DM_RESET
>> +#if (!defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM_RESET)) ||        \
>> +    (defined(CONFIG_SPL_RESET_SUPPORT) && defined(CONFIG_DM_RESET))
>
> We need option #2, thanks!
 Okay, will add this patch in new revision.

Regards
Ley Foon