diff mbox

ata: libahci: Use of_platform_device_create only if supported

Message ID 1422721938-1542-1-git-send-email-linux@roeck-us.net
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Guenter Roeck Jan. 31, 2015, 4:32 p.m. UTC
of_platform_device_create does not exist if OF_ADDRESS is not configured,
so limit its use accordingly.

Without this fix, the sparc64:allmodconfig build fails with

ERROR: "of_platform_device_create" [drivers/ata/libahci_platform.ko] undefined!

Fixes: c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Applies to -next.

 drivers/ata/libahci_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Hans de Goede Jan. 31, 2015, 4:34 p.m. UTC | #1
Hi,

On 01/31/2015 05:32 PM, Guenter Roeck wrote:
> of_platform_device_create does not exist if OF_ADDRESS is not configured,
> so limit its use accordingly.
> 
> Without this fix, the sparc64:allmodconfig build fails with
> 
> ERROR: "of_platform_device_create" [drivers/ata/libahci_platform.ko] undefined!
> 
> Fixes: c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Looks good, Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
> Applies to -next.
> 
>  drivers/ata/libahci_platform.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 077c7a2..de9a5dc 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -434,7 +434,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
>  			}
>  			mask_port_map |= BIT(port);
>  
> -			of_platform_device_create(child, NULL, NULL);
> +			if (IS_ENABLED(CONFIG_OF_ADDRESS))
> +				of_platform_device_create(child, NULL, NULL);
>  
>  			port_dev = of_find_device_by_node(child);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Jan. 31, 2015, 4:45 p.m. UTC | #2
On Sat, Jan 31, 2015 at 08:32:18AM -0800, Guenter Roeck wrote:
> of_platform_device_create does not exist if OF_ADDRESS is not configured,
> so limit its use accordingly.
> 
> Without this fix, the sparc64:allmodconfig build fails with
> 
> ERROR: "of_platform_device_create" [drivers/ata/libahci_platform.ko] undefined!
> 
> Fixes: c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Applies to -next.
> 
>  drivers/ata/libahci_platform.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 077c7a2..de9a5dc 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -434,7 +434,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
>  			}
>  			mask_port_map |= BIT(port);
>  
> -			of_platform_device_create(child, NULL, NULL);
> +			if (IS_ENABLED(CONFIG_OF_ADDRESS))
> +				of_platform_device_create(child, NULL, NULL);

This is weird.  Please either define a proper dummy version of the
function in the appropriate header file or take out the whole
invocation section w/ #ifndef.  The above is a weird hybrid where both
the declaration and the user here are doing things half-way.  Please
don't do this in other places either.

Thanks.
Guenter Roeck Jan. 31, 2015, 4:53 p.m. UTC | #3
On 01/31/2015 08:45 AM, Tejun Heo wrote:
> On Sat, Jan 31, 2015 at 08:32:18AM -0800, Guenter Roeck wrote:
>> of_platform_device_create does not exist if OF_ADDRESS is not configured,
>> so limit its use accordingly.
>>
>> Without this fix, the sparc64:allmodconfig build fails with
>>
>> ERROR: "of_platform_device_create" [drivers/ata/libahci_platform.ko] undefined!
>>
>> Fixes: c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> Applies to -next.
>>
>>   drivers/ata/libahci_platform.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
>> index 077c7a2..de9a5dc 100644
>> --- a/drivers/ata/libahci_platform.c
>> +++ b/drivers/ata/libahci_platform.c
>> @@ -434,7 +434,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
>>   			}
>>   			mask_port_map |= BIT(port);
>>
>> -			of_platform_device_create(child, NULL, NULL);
>> +			if (IS_ENABLED(CONFIG_OF_ADDRESS))
>> +				of_platform_device_create(child, NULL, NULL);
>
> This is weird.  Please either define a proper dummy version of the
> function in the appropriate header file or take out the whole
> invocation section w/ #ifndef.  The above is a weird hybrid where both
> the declaration and the user here are doing things half-way.  Please
> don't do this in other places either.
>

I agree, it is weird, as is the whole "all API functions associated
with OF_ADDRESS are handled differently on sparc" thing. But that
is neither new nor something I can fix.

No problem, I'll resubmit with #ifndef.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Jan. 31, 2015, 5:08 p.m. UTC | #4
On Sat, Jan 31, 2015 at 08:53:36AM -0800, Guenter Roeck wrote:
> I agree, it is weird, as is the whole "all API functions associated
> with OF_ADDRESS are handled differently on sparc" thing. But that
> is neither new nor something I can fix.

Out of curiousity, why isn't the header file doing the following?

#ifdef CONFIG_OF_ADDRESS

void of_platform_device_create(blah);
...

#else

static inline void of_platform_device_create(blah)
{
}

#endif
Guenter Roeck Jan. 31, 2015, 5:33 p.m. UTC | #5
On 01/31/2015 09:08 AM, Tejun Heo wrote:
> On Sat, Jan 31, 2015 at 08:53:36AM -0800, Guenter Roeck wrote:
>> I agree, it is weird, as is the whole "all API functions associated
>> with OF_ADDRESS are handled differently on sparc" thing. But that
>> is neither new nor something I can fix.
>
> Out of curiousity, why isn't the header file doing the following?
>
> #ifdef CONFIG_OF_ADDRESS
>
> void of_platform_device_create(blah);
> ...
>
> #else
>
> static inline void of_platform_device_create(blah)
> {
> }
>
> #endif
>

Maybe it is to ensure that the callers realize that the code won't
always work as intended, but I don't really know. It is kind of annoying,
though, since the resulting compile errors show up on a regular basis.
Mostly they are resolved by disabling the affected code for sparc,
but obviously that won't work here.

AFAIK this only affects sparc, all other architectures support OF_ADDRESS.

There was some discussion to get rid of OF_ADDRESS at some point,
so there is hope that this will get resolved. Until then all we can do
is to put in bandages.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 077c7a2..de9a5dc 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -434,7 +434,8 @@  struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 			}
 			mask_port_map |= BIT(port);
 
-			of_platform_device_create(child, NULL, NULL);
+			if (IS_ENABLED(CONFIG_OF_ADDRESS))
+				of_platform_device_create(child, NULL, NULL);
 
 			port_dev = of_find_device_by_node(child);