Re: [PATCH] m68knommu: include SDHC support only when hardware has it

[Greg Ungerer <gerg@linux-m68k.org>]

Hi Angelo,

On 21/9/20 5:38 pm, Angelo Dureghello wrote:
> Hi Greg,
> 
> thanks a lot for the fix.
> 
> On Mon, Sep 21, 2020 at 5:09 AM Greg Ungerer <gerg@linux-m68k.org> wrote:
>>
>> The mere fact that the kernel has the MMC subsystem enabled (CONFIG_MMC
>> enabled) does not mean that the underlying hardware platform has the
>> SDHC hardware present. Within the ColdFire hardware defines that is
>> signified by MCFSDHC_BASE being defined with an address.
>>
>> The platform data for the ColdFire parts is including the SDHC hardware
>> if CONFIG_MMC is enabled, instead of MCFSDHC_BASE. This means that if
>> you are compiling for a ColdFire target that does not support SDHC but
>> enable CONFIG_MMC you will fail to compile with errors like this:
>>
>>      arch/m68k/coldfire/device.c:565:12: error: ‘MCFSDHC_BASE’ undeclared here (not in a function)
>>         .start = MCFSDHC_BASE,
>>              ^
>>      arch/m68k/coldfire/device.c:566:25: error: ‘MCFSDHC_SIZE’ undeclared here (not in a function)
>>         .end = MCFSDHC_BASE + MCFSDHC_SIZE - 1,
>>                           ^
>>      arch/m68k/coldfire/device.c:569:12: error: ‘MCF_IRQ_SDHC’ undeclared here (not in a function)
>>         .start = MCF_IRQ_SDHC,
>>              ^
>>
>> Make the SDHC platform support depend on MCFSDHC_BASE, that is only
>> include it if the specific ColdFire SoC has that hardware module.
>>
>> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
>> ---
>>   arch/m68k/coldfire/device.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
>> index 9ef4ec0aea00..59f7dfe50a4d 100644
>> --- a/arch/m68k/coldfire/device.c
>> +++ b/arch/m68k/coldfire/device.c
>> @@ -554,7 +554,7 @@ static struct platform_device mcf_edma = {
>>   };
>>   #endif /* IS_ENABLED(CONFIG_MCF_EDMA) */
>>
>> -#if IS_ENABLED(CONFIG_MMC)
>> +#ifdef MCFSDHC_BASE
>>   static struct mcf_esdhc_platform_data mcf_esdhc_data = {
>>          .max_bus_width = 4,
>>          .cd_type = ESDHC_CD_NONE,
>> @@ -579,7 +579,7 @@ static struct platform_device mcf_esdhc = {
>>          .resource               = mcf_esdhc_resources,
>>          .dev.platform_data      = &mcf_esdhc_data,
>>   };
>> -#endif /* IS_ENABLED(CONFIG_MMC) */
>> +#endif /* MCFSDHC_BASE */
>>
>>   static struct platform_device *mcf_devices[] __initdata = {
>>          &mcf_uart,
>> @@ -613,7 +613,7 @@ static struct platform_device *mcf_devices[] __initdata = {
>>   #if IS_ENABLED(CONFIG_MCF_EDMA)
>>          &mcf_edma,
>>   #endif
>> -#if IS_ENABLED(CONFIG_MMC)
>> +#ifdef MCFSDHC_BASE
>>          &mcf_esdhc,
>>   #endif
>>   };
>> --
>> 2.25.1
>>
> 
> Tested on mcf54418 stmark2.
> 
> Reviewed-by: Angelo Dureghello <angelo.dureghello@timesys.com>
> Tested-by: Angelo Dureghello <angelo.dureghello@timesys.com>

Thanks, will add those.

Regards
Greg