After the commit 3a7a2ab839ad couple of functions became a dead code.
Moreover, for all these years nobody used them. Remove.
Fixes: 3a7a2ab839ad ("ACPI / property: Extend fwnode_property_* to data-only subnodes")
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/property.c | 20 --------------------
include/linux/acpi.h | 21 ---------------------
2 files changed, 41 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 16b28084c1ca..22ccab4e7c6d 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -841,20 +841,6 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
return ret;
}
-int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
- enum dev_prop_type proptype, void *val)
-{
- int ret;
-
- if (!adev || !val)
- return -EINVAL;
-
- ret = acpi_data_prop_read_single(&adev->data, propname, proptype, val);
- if (ret < 0 || proptype != ACPI_TYPE_STRING)
- return ret;
- return 0;
-}
-
static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
size_t nval)
{
@@ -995,12 +981,6 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
return ret;
}
-int acpi_dev_prop_read(const struct acpi_device *adev, const char *propname,
- enum dev_prop_type proptype, void *val, size_t nval)
-{
- return adev ? acpi_data_prop_read(&adev->data, propname, proptype, val, nval) : -EINVAL;
-}
-
/**
* acpi_node_prop_read - retrieve the value of an ACPI property with given name.
* @fwnode: Firmware node to get the property from.
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ea296289a94c..14ac25165ae1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1121,14 +1121,9 @@ acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
void **valptr);
-int acpi_dev_prop_read_single(struct acpi_device *adev,
- const char *propname, enum dev_prop_type proptype,
- void *val);
int acpi_node_prop_read(const struct fwnode_handle *fwnode,
const char *propname, enum dev_prop_type proptype,
void *val, size_t nval);
-int acpi_dev_prop_read(const struct acpi_device *adev, const char *propname,
- enum dev_prop_type proptype, void *val, size_t nval);
struct fwnode_handle *acpi_get_next_subnode(const struct fwnode_handle *fwnode,
struct fwnode_handle *child);
@@ -1230,14 +1225,6 @@ static inline int acpi_node_prop_get(const struct fwnode_handle *fwnode,
return -ENXIO;
}
-static inline int acpi_dev_prop_read_single(const struct acpi_device *adev,
- const char *propname,
- enum dev_prop_type proptype,
- void *val)
-{
- return -ENXIO;
-}
-
static inline int acpi_node_prop_read(const struct fwnode_handle *fwnode,
const char *propname,
enum dev_prop_type proptype,
@@ -1246,14 +1233,6 @@ static inline int acpi_node_prop_read(const struct fwnode_handle *fwnode,
return -ENXIO;
}
-static inline int acpi_dev_prop_read(const struct acpi_device *adev,
- const char *propname,
- enum dev_prop_type proptype,
- void *val, size_t nval)
-{
- return -ENXIO;
-}
-
static inline struct fwnode_handle *
acpi_get_next_subnode(const struct fwnode_handle *fwnode,
struct fwnode_handle *child)
--
2.30.0
Refactor acpi_data_prop_read_single() for less LOCs and better maintenance.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/property.c | 80 ++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 46 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index e312ebaed8db..494cf283a573 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -785,60 +785,48 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
enum dev_prop_type proptype, void *val)
{
const union acpi_object *obj;
- int ret;
+ int ret = 0;
- if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
+ if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64)
ret = acpi_data_get_property(data, propname, ACPI_TYPE_INTEGER, &obj);
- if (ret)
- return ret;
-
- switch (proptype) {
- case DEV_PROP_U8:
- if (obj->integer.value > U8_MAX)
- return -EOVERFLOW;
-
- if (val)
- *(u8 *)val = obj->integer.value;
-
- break;
- case DEV_PROP_U16:
- if (obj->integer.value > U16_MAX)
- return -EOVERFLOW;
-
- if (val)
- *(u16 *)val = obj->integer.value;
-
- break;
- case DEV_PROP_U32:
- if (obj->integer.value > U32_MAX)
- return -EOVERFLOW;
-
- if (val)
- *(u32 *)val = obj->integer.value;
-
- break;
- default:
- if (val)
- *(u64 *)val = obj->integer.value;
-
- break;
- }
-
- if (!val)
- return 1;
- } else if (proptype == DEV_PROP_STRING) {
+ else if (proptype == DEV_PROP_STRING)
ret = acpi_data_get_property(data, propname, ACPI_TYPE_STRING, &obj);
- if (ret)
- return ret;
+ if (ret)
+ return ret;
+ switch (proptype) {
+ case DEV_PROP_U8:
+ if (obj->integer.value > U8_MAX)
+ return -EOVERFLOW;
+ if (val)
+ *(u8 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_U16:
+ if (obj->integer.value > U16_MAX)
+ return -EOVERFLOW;
+ if (val)
+ *(u16 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_U32:
+ if (obj->integer.value > U32_MAX)
+ return -EOVERFLOW;
+ if (val)
+ *(u32 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_U64:
+ if (val)
+ *(u64 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_STRING:
if (val)
*(char **)val = obj->string.pointer;
-
return 1;
- } else {
- ret = -EINVAL;
+ default:
+ return -EINVAL;
}
- return ret;
+
+ /* When no storage provided return number of available values */
+ return val ? 0 : 1;
}
static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
--
2.30.0
CHECK drivers/acpi/property.c
warning: Function parameter or member 'data' not described in 'acpi_data_get_property_array'
warning: Excess function parameter 'adev' description in 'acpi_data_get_property_array'
Fixes: 3a7a2ab839ad ("ACPI / property: Extend fwnode_property_* to data-only subnodes")
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/property.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 2b65ad9b4c0d..ab4d7c734b0d 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -564,7 +564,7 @@ int acpi_node_prop_get(const struct fwnode_handle *fwnode,
/**
* acpi_data_get_property_array - return an ACPI array property with given name
- * @adev: ACPI data object to get the property from
+ * @data: ACPI data object to get the property from
* @name: Name of the property
* @type: Expected type of array elements
* @obj: Location to store a pointer to the property value (if not NULL)
--
2.30.0
On Fri, Feb 12, 2021 at 3:14 PM Andy Shevchenko
<[email protected]> wrote:
>
> Refactor acpi_data_prop_read_single() for less LOCs and better maintenance.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/acpi/property.c | 80 ++++++++++++++++++-----------------------
> 1 file changed, 34 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index e312ebaed8db..494cf283a573 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -785,60 +785,48 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
> enum dev_prop_type proptype, void *val)
> {
> const union acpi_object *obj;
> - int ret;
> + int ret = 0;
>
> - if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
> + if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64)
> ret = acpi_data_get_property(data, propname, ACPI_TYPE_INTEGER, &obj);
> - if (ret)
> - return ret;
> -
> - switch (proptype) {
> - case DEV_PROP_U8:
> - if (obj->integer.value > U8_MAX)
> - return -EOVERFLOW;
> -
> - if (val)
> - *(u8 *)val = obj->integer.value;
> -
> - break;
The empty lines of code above are intentional, so please retain them.
> - case DEV_PROP_U16:
> - if (obj->integer.value > U16_MAX)
> - return -EOVERFLOW;
> -
> - if (val)
> - *(u16 *)val = obj->integer.value;
> -
> - break;
> - case DEV_PROP_U32:
> - if (obj->integer.value > U32_MAX)
> - return -EOVERFLOW;
> -
> - if (val)
> - *(u32 *)val = obj->integer.value;
> -
> - break;
> - default:
> - if (val)
> - *(u64 *)val = obj->integer.value;
> -
> - break;
> - }
> -
> - if (!val)
> - return 1;
> - } else if (proptype == DEV_PROP_STRING) {
> + else if (proptype == DEV_PROP_STRING)
> ret = acpi_data_get_property(data, propname, ACPI_TYPE_STRING, &obj);
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
else if (!val)
ret = 1;
>
> + switch (proptype) {
> + case DEV_PROP_U8:
> + if (obj->integer.value > U8_MAX)
> + return -EOVERFLOW;
> + if (val)
> + *(u8 *)val = obj->integer.value;
> + break;
> + case DEV_PROP_U16:
> + if (obj->integer.value > U16_MAX)
> + return -EOVERFLOW;
> + if (val)
> + *(u16 *)val = obj->integer.value;
> + break;
> + case DEV_PROP_U32:
> + if (obj->integer.value > U32_MAX)
> + return -EOVERFLOW;
> + if (val)
> + *(u32 *)val = obj->integer.value;
> + break;
> + case DEV_PROP_U64:
> + if (val)
> + *(u64 *)val = obj->integer.value;
> + break;
> + case DEV_PROP_STRING:
> if (val)
> *(char **)val = obj->string.pointer;
> -
> return 1;
> - } else {
> - ret = -EINVAL;
> + default:
> + return -EINVAL;
> }
> - return ret;
Retain this.
> +
> + /* When no storage provided return number of available values */
> + return val ? 0 : 1;
And this is just not looking good to me, sorry.
> }
>
> static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
> --
On Fri, Feb 12, 2021 at 3:16 PM Andy Shevchenko
<[email protected]> wrote:
>
> After the commit 3a7a2ab839ad couple of functions became a dead code.
> Moreover, for all these years nobody used them. Remove.
>
> Fixes: 3a7a2ab839ad ("ACPI / property: Extend fwnode_property_* to data-only subnodes")
> Signed-off-by: Andy Shevchenko <[email protected]>
Patches [1-4/5] from this series applied as 5.12 material and please
see my comments for the last one sent separately.
Thanks!
> ---
> drivers/acpi/property.c | 20 --------------------
> include/linux/acpi.h | 21 ---------------------
> 2 files changed, 41 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 16b28084c1ca..22ccab4e7c6d 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -841,20 +841,6 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data,
> return ret;
> }
>
> -int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
> - enum dev_prop_type proptype, void *val)
> -{
> - int ret;
> -
> - if (!adev || !val)
> - return -EINVAL;
> -
> - ret = acpi_data_prop_read_single(&adev->data, propname, proptype, val);
> - if (ret < 0 || proptype != ACPI_TYPE_STRING)
> - return ret;
> - return 0;
> -}
> -
> static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
> size_t nval)
> {
> @@ -995,12 +981,6 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
> return ret;
> }
>
> -int acpi_dev_prop_read(const struct acpi_device *adev, const char *propname,
> - enum dev_prop_type proptype, void *val, size_t nval)
> -{
> - return adev ? acpi_data_prop_read(&adev->data, propname, proptype, val, nval) : -EINVAL;
> -}
> -
> /**
> * acpi_node_prop_read - retrieve the value of an ACPI property with given name.
> * @fwnode: Firmware node to get the property from.
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index ea296289a94c..14ac25165ae1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1121,14 +1121,9 @@ acpi_data_add_props(struct acpi_device_data *data, const guid_t *guid,
>
> int acpi_node_prop_get(const struct fwnode_handle *fwnode, const char *propname,
> void **valptr);
> -int acpi_dev_prop_read_single(struct acpi_device *adev,
> - const char *propname, enum dev_prop_type proptype,
> - void *val);
> int acpi_node_prop_read(const struct fwnode_handle *fwnode,
> const char *propname, enum dev_prop_type proptype,
> void *val, size_t nval);
> -int acpi_dev_prop_read(const struct acpi_device *adev, const char *propname,
> - enum dev_prop_type proptype, void *val, size_t nval);
>
> struct fwnode_handle *acpi_get_next_subnode(const struct fwnode_handle *fwnode,
> struct fwnode_handle *child);
> @@ -1230,14 +1225,6 @@ static inline int acpi_node_prop_get(const struct fwnode_handle *fwnode,
> return -ENXIO;
> }
>
> -static inline int acpi_dev_prop_read_single(const struct acpi_device *adev,
> - const char *propname,
> - enum dev_prop_type proptype,
> - void *val)
> -{
> - return -ENXIO;
> -}
> -
> static inline int acpi_node_prop_read(const struct fwnode_handle *fwnode,
> const char *propname,
> enum dev_prop_type proptype,
> @@ -1246,14 +1233,6 @@ static inline int acpi_node_prop_read(const struct fwnode_handle *fwnode,
> return -ENXIO;
> }
>
> -static inline int acpi_dev_prop_read(const struct acpi_device *adev,
> - const char *propname,
> - enum dev_prop_type proptype,
> - void *val, size_t nval)
> -{
> - return -ENXIO;
> -}
> -
> static inline struct fwnode_handle *
> acpi_get_next_subnode(const struct fwnode_handle *fwnode,
> struct fwnode_handle *child)
> --
> 2.30.0
>
On Fri, Feb 12, 2021 at 03:31:24PM +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 12, 2021 at 3:14 PM Andy Shevchenko
> <[email protected]> wrote:
> > Refactor acpi_data_prop_read_single() for less LOCs and better maintenance.
Thanks for review, my answers below.
...
> > + if (ret)
> > + return ret;
>
> else if (!val)
> ret = 1;
But then it become a bug again :-)
...
> And this is just not looking good to me, sorry.
Yeah, I think this patch is not needed right now. At least it seems no gain
from it.
--
With Best Regards,
Andy Shevchenko
On Fri, Feb 12, 2021 at 5:01 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Feb 12, 2021 at 03:31:24PM +0100, Rafael J. Wysocki wrote:
> > On Fri, Feb 12, 2021 at 3:14 PM Andy Shevchenko
> > <[email protected]> wrote:
>
> > > Refactor acpi_data_prop_read_single() for less LOCs and better maintenance.
>
> Thanks for review, my answers below.
>
> ...
>
> > > + if (ret)
> > > + return ret;
> >
> > else if (!val)
> > ret = 1;
>
> But then it become a bug again :-)
I'm not sure why? The checks below will still happen and they may
cause an error to be returned.
> ...
>
> > And this is just not looking good to me, sorry.
>
> Yeah, I think this patch is not needed right now. At least it seems no gain
> from it.
OK
On Fri, Feb 12, 2021 at 6:11 PM Rafael J. Wysocki <[email protected]> wrote:
> On Fri, Feb 12, 2021 at 5:01 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, Feb 12, 2021 at 03:31:24PM +0100, Rafael J. Wysocki wrote:
> > > On Fri, Feb 12, 2021 at 3:14 PM Andy Shevchenko
> > > <[email protected]> wrote:
...
> > > > + if (ret)
> > > > + return ret;
> > >
> > > else if (!val)
> > > ret = 1;
> >
> > But then it become a bug again :-)
>
> I'm not sure why? The checks below will still happen and they may
> cause an error to be returned.
Oh, I misinterpreted ret = as plain return. Right. Seems okay.
--
With Best Regards,
Andy Shevchenko