Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200306 > unrolled thread

[PATCH] acpi, property: Export acpi_dev_prop_read_single call.

Started byDavid Daney <ddaney.cavm@gmail.com>
First post2015-08-05 01:10 +0200
Last post2015-08-06 02:00 +0200
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] acpi, property: Export acpi_dev_prop_read_single call. David Daney <ddaney.cavm@gmail.com> - 2015-08-05 01:10 +0200
    Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-08-05 15:30 +0200
      Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. Tomasz Nowicki <tomasz.nowicki@linaro.org> - 2015-08-05 15:50 +0200
        Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. David Daney <ddaney@caviumnetworks.com> - 2015-08-05 19:30 +0200
          Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. David Daney <ddaney.cavm@gmail.com> - 2015-08-05 22:20 +0200
            Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-08-06 01:00 +0200
              Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-08-06 01:20 +0200
              Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-08-06 01:30 +0200
                Re: [PATCH] acpi, property: Export acpi_dev_prop_read_single call. David Daney <ddaney.cavm@gmail.com> - 2015-08-06 02:00 +0200

#1200306 — [PATCH] acpi, property: Export acpi_dev_prop_read_single call.

FromDavid Daney <ddaney.cavm@gmail.com>
Date2015-08-05 01:10 +0200
Subject[PATCH] acpi, property: Export acpi_dev_prop_read_single call.
Message-ID<pTTvj-8pp-7@gated-at.bofh.it>
From: Tomasz Nowicki <tomasz.nowicki@linaro.org>

Fixes the following build error when building drivers as modules:

 ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko] undefined!
 ERROR: "acpi_dev_prop_read_single" [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!

Reported-by: Andreas Schwab <schwab@suse.de>
Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
Signed-off-by: Robert Richter <rrichter@cavium.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/acpi/property.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 7836e2e..237e3c5 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -432,6 +432,7 @@ int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
 	}
 	return ret;
 }
+EXPORT_SYMBOL_GPL(acpi_dev_prop_read_single);
 
 static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
 				       size_t nval)
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1200736

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-08-05 15:30 +0200
Message-ID<pU6VC-2zD-37@gated-at.bofh.it>
In reply to#1200306
On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> 
> Fixes the following build error when building drivers as modules:
> 
>  ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko] undefined!
>  ERROR: "acpi_dev_prop_read_single" [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!

Can you please tell me why the drivers in question use that function
directly, although they aren't supposed to?

Clearly, their authors had not tried to build them as modules or they
would have noticed the problem at the development stage already.

What would be wrong with using the generic device properties API instead?


> Reported-by: Andreas Schwab <schwab@suse.de>
> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/acpi/property.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 7836e2e..237e3c5 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -432,6 +432,7 @@ int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
>  	}
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(acpi_dev_prop_read_single);
>  
>  static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
>  				       size_t nval)
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200759

FromTomasz Nowicki <tomasz.nowicki@linaro.org>
Date2015-08-05 15:50 +0200
Message-ID<pU7eW-2WR-19@gated-at.bofh.it>
In reply to#1200736
On 05.08.2015 15:48, Rafael J. Wysocki wrote:
> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>>
>> Fixes the following build error when building drivers as modules:
>>
>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko] undefined!
>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
>
> Can you please tell me why the drivers in question use that function
> directly, although they aren't supposed to?
>
> Clearly, their authors had not tried to build them as modules or they
> would have noticed the problem at the development stage already.
>
> What would be wrong with using the generic device properties API instead?
>
Yes, you are right. We should use:
int device_property_read_u64_array(struct device *dev, const char 
*propname, u64 *val, size_t nval);

Regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201015

FromDavid Daney <ddaney@caviumnetworks.com>
Date2015-08-05 19:30 +0200
Message-ID<pUaFQ-811-17@gated-at.bofh.it>
In reply to#1200759
On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>>>
>>> Fixes the following build error when building drivers as modules:
>>>
>>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
>>> undefined!
>>>   ERROR: "acpi_dev_prop_read_single"
>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
>>
>> Can you please tell me why the drivers in question use that function
>> directly, although they aren't supposed to?
>>
>> Clearly, their authors had not tried to build them as modules or they
>> would have noticed the problem at the development stage already.
>>
>> What would be wrong with using the generic device properties API instead?
>>
> Yes, you are right. We should use:
> int device_property_read_u64_array(struct device *dev, const char
> *propname, u64 *val, size_t nval);
>

Thanks all, for the review and suggestions.  We we try the suggested 
approach and see how it goes...


Thanks again,
David Daney

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201111

FromDavid Daney <ddaney.cavm@gmail.com>
Date2015-08-05 22:20 +0200
Message-ID<pUdkn-3qW-43@gated-at.bofh.it>
In reply to#1201015
On 08/05/2015 10:26 AM, David Daney wrote:
> On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
>> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
>>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
>>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>>>>
>>>> Fixes the following build error when building drivers as modules:
>>>>
>>>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
>>>> undefined!
>>>>   ERROR: "acpi_dev_prop_read_single"
>>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
>>>
>>> Can you please tell me why the drivers in question use that function
>>> directly, although they aren't supposed to?
>>>
>>> Clearly, their authors had not tried to build them as modules or they
>>> would have noticed the problem at the development stage already.
>>>
>>> What would be wrong with using the generic device properties API
>>> instead?
>>>
>> Yes, you are right. We should use:
>> int device_property_read_u64_array(struct device *dev, const char
>> *propname, u64 *val, size_t nval);
>>
>
> Thanks all, for the review and suggestions.  We we try the suggested
> approach and see how it goes...
>

Actually I don't think device_property_read_u64_array() will work.

We are traversing a reference to a different acpi_device via 
acpi_dev_get_property_reference(), so there is no struct device * 
available for a call to device_property_read_u64_array().  This looks 
like a deficiency in the device_property_* framework, so for the time 
being I guess we will call acpi_dev_get_property(), which is exported, 
and decode the thing in the driver.

David Daney


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201325

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-08-06 01:00 +0200
Message-ID<pUfPb-6TO-1@gated-at.bofh.it>
In reply to#1201111
On Wednesday, August 05, 2015 01:14:49 PM David Daney wrote:
> On 08/05/2015 10:26 AM, David Daney wrote:
> > On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
> >> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
> >>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
> >>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> >>>>
> >>>> Fixes the following build error when building drivers as modules:
> >>>>
> >>>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
> >>>> undefined!
> >>>>   ERROR: "acpi_dev_prop_read_single"
> >>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
> >>>
> >>> Can you please tell me why the drivers in question use that function
> >>> directly, although they aren't supposed to?
> >>>
> >>> Clearly, their authors had not tried to build them as modules or they
> >>> would have noticed the problem at the development stage already.
> >>>
> >>> What would be wrong with using the generic device properties API
> >>> instead?
> >>>
> >> Yes, you are right. We should use:
> >> int device_property_read_u64_array(struct device *dev, const char
> >> *propname, u64 *val, size_t nval);
> >>
> >
> > Thanks all, for the review and suggestions.  We we try the suggested
> > approach and see how it goes...
> >
> 
> Actually I don't think device_property_read_u64_array() will work.
> 
> We are traversing a reference to a different acpi_device via 
> acpi_dev_get_property_reference(),

Why?

> so there is no struct device * 
> available for a call to device_property_read_u64_array().  This looks 
> like a deficiency in the device_property_* framework, so for the time 
> being I guess we will call acpi_dev_get_property(), which is exported, 
> and decode the thing in the driver.

Please don't.

I'd like to understand what's missing.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201334

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-08-06 01:20 +0200
Message-ID<pUg8x-7vP-5@gated-at.bofh.it>
In reply to#1201325
On Thursday, August 06, 2015 01:23:06 AM Rafael J. Wysocki wrote:
> On Wednesday, August 05, 2015 01:14:49 PM David Daney wrote:
> > On 08/05/2015 10:26 AM, David Daney wrote:
> > > On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
> > >> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
> > >>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
> > >>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> > >>>>
> > >>>> Fixes the following build error when building drivers as modules:
> > >>>>
> > >>>>   ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
> > >>>> undefined!
> > >>>>   ERROR: "acpi_dev_prop_read_single"
> > >>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
> > >>>
> > >>> Can you please tell me why the drivers in question use that function
> > >>> directly, although they aren't supposed to?
> > >>>
> > >>> Clearly, their authors had not tried to build them as modules or they
> > >>> would have noticed the problem at the development stage already.
> > >>>
> > >>> What would be wrong with using the generic device properties API
> > >>> instead?
> > >>>
> > >> Yes, you are right. We should use:
> > >> int device_property_read_u64_array(struct device *dev, const char
> > >> *propname, u64 *val, size_t nval);
> > >>
> > >
> > > Thanks all, for the review and suggestions.  We we try the suggested
> > > approach and see how it goes...
> > >
> > 
> > Actually I don't think device_property_read_u64_array() will work.
> > 
> > We are traversing a reference to a different acpi_device via 
> > acpi_dev_get_property_reference(),
> 
> Why?
> 
> > so there is no struct device * 
> > available for a call to device_property_read_u64_array().  This looks 
> > like a deficiency in the device_property_* framework, so for the time 
> > being I guess we will call acpi_dev_get_property(), which is exported, 
> > and decode the thing in the driver.
> 
> Please don't.
> 
> I'd like to understand what's missing.

Moreover, even if you have no struct device, you still can use
fwnode_property_read_u64_array(), can't you?

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201336

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-08-06 01:30 +0200
Message-ID<pUgid-7Hc-1@gated-at.bofh.it>
In reply to#1201325
On Wednesday, August 05, 2015 04:17:21 PM David Daney wrote:
> On 08/05/2015 04:23 PM, Rafael J. Wysocki wrote:
> > On Wednesday, August 05, 2015 01:14:49 PM David Daney wrote:
> >> On 08/05/2015 10:26 AM, David Daney wrote:
> >>> On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
> >>>> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
> >>>>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
> >>>>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> >>>>>>
> >>>>>> Fixes the following build error when building drivers as modules:
> >>>>>>
> >>>>>>    ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
> >>>>>> undefined!
> >>>>>>    ERROR: "acpi_dev_prop_read_single"
> >>>>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
> >>>>>
> >>>>> Can you please tell me why the drivers in question use that function
> >>>>> directly, although they aren't supposed to?
> >>>>>
> >>>>> Clearly, their authors had not tried to build them as modules or they
> >>>>> would have noticed the problem at the development stage already.
> >>>>>
> >>>>> What would be wrong with using the generic device properties API
> >>>>> instead?
> >>>>>
> >>>> Yes, you are right. We should use:
> >>>> int device_property_read_u64_array(struct device *dev, const char
> >>>> *propname, u64 *val, size_t nval);
> >>>>
> >>>
> >>> Thanks all, for the review and suggestions.  We we try the suggested
> >>> approach and see how it goes...
> >>>
> >>
> >> Actually I don't think device_property_read_u64_array() will work.
> >>
> >> We are traversing a reference to a different acpi_device via
> >> acpi_dev_get_property_reference(),
> >
> > Why?
> 
> Network device has a "phy-handle" (traversed with 
> acpi_dev_get_property_reference()), and we want to get some properties 
> of that phy.
> 
> I could turn the question around to you:  Why export 
> acpi_dev_get_property_reference()?  If there is a reason to export that, 
> then you should let people use the result.

The GPIO core uses it and you *can* use the result (please see my other
message).

I wonder how does the ACPI table in question look like.  Do you have
an acpidump output from that system by any chance?

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201350

FromDavid Daney <ddaney.cavm@gmail.com>
Date2015-08-06 02:00 +0200
Message-ID<pUgLg-8fz-7@gated-at.bofh.it>
In reply to#1201336
On 08/05/2015 04:55 PM, Rafael J. Wysocki wrote:
> On Wednesday, August 05, 2015 04:17:21 PM David Daney wrote:
>> On 08/05/2015 04:23 PM, Rafael J. Wysocki wrote:
>>> On Wednesday, August 05, 2015 01:14:49 PM David Daney wrote:
>>>> On 08/05/2015 10:26 AM, David Daney wrote:
>>>>> On 08/05/2015 06:43 AM, Tomasz Nowicki wrote:
>>>>>> On 05.08.2015 15:48, Rafael J. Wysocki wrote:
>>>>>>> On Tuesday, August 04, 2015 04:01:59 PM David Daney wrote:
>>>>>>>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>>>>>>>>
>>>>>>>> Fixes the following build error when building drivers as modules:
>>>>>>>>
>>>>>>>>     ERROR: "acpi_dev_prop_read_single" [drivers/net/phy/mdio-octeon.ko]
>>>>>>>> undefined!
>>>>>>>>     ERROR: "acpi_dev_prop_read_single"
>>>>>>>> [drivers/net/ethernet/cavium/thunder/thunder_bgx.ko] undefined!
>>>>>>>
>>>>>>> Can you please tell me why the drivers in question use that function
>>>>>>> directly, although they aren't supposed to?
>>>>>>>
>>>>>>> Clearly, their authors had not tried to build them as modules or they
>>>>>>> would have noticed the problem at the development stage already.
>>>>>>>
>>>>>>> What would be wrong with using the generic device properties API
>>>>>>> instead?
>>>>>>>
>>>>>> Yes, you are right. We should use:
>>>>>> int device_property_read_u64_array(struct device *dev, const char
>>>>>> *propname, u64 *val, size_t nval);
>>>>>>
>>>>>
>>>>> Thanks all, for the review and suggestions.  We we try the suggested
>>>>> approach and see how it goes...
>>>>>
>>>>
>>>> Actually I don't think device_property_read_u64_array() will work.
>>>>
>>>> We are traversing a reference to a different acpi_device via
>>>> acpi_dev_get_property_reference(),
>>>
>>> Why?
>>
>> Network device has a "phy-handle" (traversed with
>> acpi_dev_get_property_reference()), and we want to get some properties
>> of that phy.
>>
>> I could turn the question around to you:  Why export
>> acpi_dev_get_property_reference()?  If there is a reason to export that,
>> then you should let people use the result.
>
> The GPIO core uses it and you *can* use the result (please see my other
> message).

It was somewhat of a rhetorical question.

I would actually like to do as you suggest, so I am looking at unwinding 
the driver code to use the interfaces you suggest.

It is a multi-step process for me though.  First I had to make the 
existing code work.  I now have it working, so next I will try to fix it.

>
> I wonder how does the ACPI table in question look like.  Do you have
> an acpidump output from that system by any chance?

Not handy.  I will be producing one soon though.

The good news is that I can change the firmware to make it correct if it 
has problems.


Thanks for your patience,
David Daney


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web