Re: [PATCH] firmware: vpd: Add an interface to read VPD value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/7/19 1:03 AM, Tzung-Bi Shih wrote:
On Mon, Oct 7, 2019 at 3:16 PM Cheng-Yi Chiang <cychiang@xxxxxxxxxxxx> wrote:

Add an interface for other driver to query VPD value.
This will be used for ASoC machine driver to query calibration
data stored in VPD for smart amplifier speaker resistor
calibration.

Signed-off-by: Cheng-Yi Chiang <cychiang@xxxxxxxxxxxx>
---
  drivers/firmware/google/vpd.c              | 16 ++++++++++++++++
  include/linux/firmware/google/google_vpd.h | 18 ++++++++++++++++++
  2 files changed, 34 insertions(+)
  create mode 100644 include/linux/firmware/google/google_vpd.h

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index db0812263d46..71e9d2da63be 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -65,6 +65,22 @@ static ssize_t vpd_attrib_read(struct file *filp, struct kobject *kobp,
                                        info->bin_attr.size);
  }

+int vpd_attribute_read_value(bool ro, const char *key,
+                            char **value, u32 value_len)

FWIW, I don't think the "_value" in this function name adds any value,
unless there is going to be some other read function.

The API should be documented, and state clearly that the caller must release
the returned value.

+{
+       struct vpd_attrib_info *info;
+       struct vpd_section *sec = ro ? &ro_vpd : &rw_vpd;
+
+       list_for_each_entry(info, &sec->attribs, list) {
+               if (strcmp(info->key, key) == 0) {
+                       *value = kstrndup(info->value, value_len, GFP_KERNEL);

Value is not necessary a NULL-terminated string.
kmalloc(info->bin_attr.size) and memcpy(...) would make the most
sense.

kmemdup() ?

The value_len parameter makes less sense.  It seems the caller knows
the length of the value in advance.
Suggest to change the value_len to report the length of value.  I.e.
*value_len = info->bin_attr.size;
 > Also please check the return value for memory allocation-like
functions (e.g. kstrndup, kmalloc) so that *value won't be NULL but
the function returned 0.

+                       return 0;
+               }
+       }
+       return -EINVAL;

Maybe something like -ENOENT would be more appropriate here.

+}
+EXPORT_SYMBOL(vpd_attribute_read_value);
+

I would suggest to use EXPORT_SYMBOL_GPL().

  /*
   * vpd_section_check_key_name()
   *
diff --git a/include/linux/firmware/google/google_vpd.h b/include/linux/firmware/google/google_vpd.h
new file mode 100644
index 000000000000..6f1160f28af8
--- /dev/null
+++ b/include/linux/firmware/google/google_vpd.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Google VPD interface.
+ *
+ * Copyright 2019 Google Inc.
+ */
+
+/* Interface for reading VPD value on Chrome platform. */
+
+#ifndef __GOOGLE_VPD_H
+#define __GOOGLE_VPD_H
+
+#include <linux/types.h>
+
+int vpd_attribute_read_value(bool ro, const char *key,
+                            char **value, u32 value_len);
+
+#endif  /* __GOOGLE_VPD_H */
--
2.23.0.581.g78d2f28ef7-goog



_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux