* [PATCH v3 1/4] scsi: ufs: add ioctl interface for query request
From: Gilad Broner @ 2015-02-23 8:08 UTC (permalink / raw)
To: James.Bottomley
Cc: linux-kernel, linux-scsi, linux-arm-msm, santoshsy,
linux-scsi-owner, subhashj, ygardi, draviv, Noa Rubens,
Raviv Shvili, Vinayak Holikatti, James E.J. Bottomley,
open list:ABI/API
In-Reply-To: <1424678898-3723-1-git-send-email-gbroner@codeaurora.org>
From: Dolev Raviv <draviv@codeaurora.org>
This patch exposes the ioctl interface for UFS driver via SCSI device
ioctl interface. As of now UFS driver would provide the ioctl for query
interface to connected UFS device.
Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
Signed-off-by: Noa Rubens <noag@codeaurora.org>
Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
---
drivers/scsi/ufs/ufs.h | 53 +++-------
drivers/scsi/ufs/ufshcd.c | 225 +++++++++++++++++++++++++++++++++++++++++-
include/uapi/scsi/Kbuild | 1 +
include/uapi/scsi/ufs/Kbuild | 3 +
include/uapi/scsi/ufs/ioctl.h | 57 +++++++++++
include/uapi/scsi/ufs/ufs.h | 66 +++++++++++++
6 files changed, 361 insertions(+), 44 deletions(-)
create mode 100644 include/uapi/scsi/ufs/Kbuild
create mode 100644 include/uapi/scsi/ufs/ioctl.h
create mode 100644 include/uapi/scsi/ufs/ufs.h
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 42c459a..1f023c4 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
#include <linux/mutex.h>
#include <linux/types.h>
+#include <scsi/ufs/ufs.h>
#define MAX_CDB_SIZE 16
#define GENERAL_UPIU_REQUEST_SIZE 32
@@ -71,6 +72,16 @@ enum {
UFS_UPIU_RPMB_WLUN = 0xC4,
};
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+ return (lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN));
+}
+
/*
* UFS Protocol Information Unit related definitions
*/
@@ -126,35 +137,6 @@ enum {
UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
};
-/* Flag idn for Query Requests*/
-enum flag_idn {
- QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
- QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
- QUERY_FLAG_IDN_BKOPS_EN = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
- QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
- QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
- QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
- QUERY_ATTR_IDN_EE_STATUS = 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
- QUERY_DESC_IDN_DEVICE = 0x0,
- QUERY_DESC_IDN_CONFIGURAION = 0x1,
- QUERY_DESC_IDN_UNIT = 0x2,
- QUERY_DESC_IDN_RFU_0 = 0x3,
- QUERY_DESC_IDN_INTERCONNECT = 0x4,
- QUERY_DESC_IDN_STRING = 0x5,
- QUERY_DESC_IDN_RFU_1 = 0x6,
- QUERY_DESC_IDN_GEOMETRY = 0x7,
- QUERY_DESC_IDN_POWER = 0x8,
- QUERY_DESC_IDN_MAX,
-};
-
enum desc_header_offset {
QUERY_DESC_LENGTH_OFFSET = 0x00,
QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
@@ -247,19 +229,6 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
- UPIU_QUERY_OPCODE_NOP = 0x0,
- UPIU_QUERY_OPCODE_READ_DESC = 0x1,
- UPIU_QUERY_OPCODE_WRITE_DESC = 0x2,
- UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
- UPIU_QUERY_OPCODE_WRITE_ATTR = 0x4,
- UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
- UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
- UPIU_QUERY_OPCODE_CLEAR_FLAG = 0x7,
- UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
-};
-
/* Query response result code */
enum {
QUERY_RESULT_SUCCESS = 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5d60a86..cb357f8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3,7 +3,7 @@
*
* This code is based on drivers/scsi/ufs/ufshcd.c
* Copyright (C) 2011-2013 Samsung India Software Operations
- * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
*
* Authors:
* Santosh Yaraganavi <santosh.sy@samsung.com>
@@ -39,6 +39,7 @@
#include <linux/async.h>
#include <linux/devfreq.h>
+#include <scsi/ufs/ioctl.h>
#include "ufshcd.h"
#include "unipro.h"
@@ -74,6 +75,9 @@
/* Interrupt aggregation default timeout, unit: 40us */
#define INT_AGGR_DEF_TO 0x02
+/* IOCTL opcode for command - ufs set device read only */
+#define UFS_IOCTL_BLKROSET BLKROSET
+
#define ufshcd_toggle_vreg(_dev, _vreg, _on) \
({ \
int _ret; \
@@ -1882,7 +1886,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
* Unit descriptors are only available for general purpose LUs (LUN id
* from 0 to 7) and RPMB Well known LU.
*/
- if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN))
+ if (!ufs_is_valid_unit_desc_lun(lun))
return -EOPNOTSUPP;
return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -4201,6 +4205,222 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
ufshcd_probe_hba(hba);
}
+/**
+ * ufshcd_query_ioctl - perform user read queries
+ * @hba: per-adapter instance
+ * @lun: used for lun specific queries
+ * @buffer: user space buffer for reading and submitting query data and params
+ * @return: 0 for success negative error code otherwise
+ *
+ * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
+ * It will read the opcode, idn and buf_length parameters, and, put the
+ * response in the buffer field while updating the used size in buf_length.
+ */
+static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user *buffer)
+{
+ struct ufs_ioctl_query_data *ioctl_data;
+ int err = 0;
+ int length = 0;
+ void *data_ptr;
+ bool flag;
+ u32 att;
+ u8 index;
+ u8 *desc = NULL;
+
+ ioctl_data = kzalloc(sizeof(struct ufs_ioctl_query_data), GFP_KERNEL);
+ if (!ioctl_data) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ /* extract params from user buffer */
+ err = copy_from_user(ioctl_data, buffer,
+ sizeof(struct ufs_ioctl_query_data));
+ if (err) {
+ dev_err(hba->dev,
+ "%s: Failed copying buffer from user, err %d\n",
+ __func__, err);
+ goto out_release_mem;
+ }
+
+ /* verify legal parameters & send query */
+ switch (ioctl_data->opcode) {
+ case UPIU_QUERY_OPCODE_READ_DESC:
+ switch (ioctl_data->idn) {
+ case QUERY_DESC_IDN_DEVICE:
+ case QUERY_DESC_IDN_CONFIGURAION:
+ case QUERY_DESC_IDN_INTERCONNECT:
+ case QUERY_DESC_IDN_GEOMETRY:
+ case QUERY_DESC_IDN_POWER:
+ index = 0;
+ break;
+ case QUERY_DESC_IDN_UNIT:
+ if (!ufs_is_valid_unit_desc_lun(lun)) {
+ dev_err(hba->dev,
+ "%s: No unit descriptor for lun 0x%x\n",
+ __func__, lun);
+ err = -EINVAL;
+ goto out_release_mem;
+ }
+ index = lun;
+ break;
+ default:
+ goto out_einval;
+ }
+ length = min_t(int, QUERY_DESC_MAX_SIZE,
+ ioctl_data->buf_size);
+ desc = kzalloc(length, GFP_KERNEL);
+ if (!desc) {
+ dev_err(hba->dev, "%s: Failed allocating %d bytes\n",
+ __func__, length);
+ err = -ENOMEM;
+ goto out_release_mem;
+ }
+ err = ufshcd_query_descriptor(hba, ioctl_data->opcode,
+ ioctl_data->idn, index, 0, desc, &length);
+ break;
+ case UPIU_QUERY_OPCODE_READ_ATTR:
+ switch (ioctl_data->idn) {
+ case QUERY_ATTR_IDN_BOOT_LU_EN:
+ case QUERY_ATTR_IDN_POWER_MODE:
+ case QUERY_ATTR_IDN_ACTIVE_ICC_LVL:
+ case QUERY_ATTR_IDN_OOO_DATA_EN:
+ case QUERY_ATTR_IDN_BKOPS_STATUS:
+ case QUERY_ATTR_IDN_PURGE_STATUS:
+ case QUERY_ATTR_IDN_MAX_DATA_IN:
+ case QUERY_ATTR_IDN_MAX_DATA_OUT:
+ case QUERY_ATTR_IDN_REF_CLK_FREQ:
+ case QUERY_ATTR_IDN_CONF_DESC_LOCK:
+ case QUERY_ATTR_IDN_MAX_NUM_OF_RTT:
+ case QUERY_ATTR_IDN_EE_CONTROL:
+ case QUERY_ATTR_IDN_EE_STATUS:
+ case QUERY_ATTR_IDN_SECONDS_PASSED:
+ index = 0;
+ break;
+ case QUERY_ATTR_IDN_DYN_CAP_NEEDED:
+ case QUERY_ATTR_IDN_CORR_PRG_BLK_NUM:
+ index = lun;
+ break;
+ default:
+ goto out_einval;
+ }
+ err = ufshcd_query_attr(hba, ioctl_data->opcode,
+ ioctl_data->idn, index, 0, &att);
+ break;
+ case UPIU_QUERY_OPCODE_READ_FLAG:
+ switch (ioctl_data->idn) {
+ case QUERY_FLAG_IDN_FDEVICEINIT:
+ case QUERY_FLAG_IDN_PERMANENT_WPE:
+ case QUERY_FLAG_IDN_PWR_ON_WPE:
+ case QUERY_FLAG_IDN_BKOPS_EN:
+ case QUERY_FLAG_IDN_PURGE_ENABLE:
+ case QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL:
+ case QUERY_FLAG_IDN_BUSY_RTC:
+ break;
+ default:
+ goto out_einval;
+ }
+ err = ufshcd_query_flag(hba, ioctl_data->opcode,
+ ioctl_data->idn, &flag);
+ break;
+ default:
+ goto out_einval;
+ }
+
+ if (err) {
+ dev_err(hba->dev, "%s: Query for idn %d failed\n", __func__,
+ ioctl_data->idn);
+ goto out_release_mem;
+ }
+
+ /*
+ * copy response data
+ * As we might end up reading less data then what is specified in
+ * "ioct_data->buf_size". So we are updating "ioct_data->
+ * buf_size" to what exactly we have read.
+ */
+ switch (ioctl_data->opcode) {
+ case UPIU_QUERY_OPCODE_READ_DESC:
+ ioctl_data->buf_size = min_t(int, ioctl_data->buf_size, length);
+ data_ptr = desc;
+ break;
+ case UPIU_QUERY_OPCODE_READ_ATTR:
+ ioctl_data->buf_size = sizeof(u32);
+ data_ptr = &att;
+ break;
+ case UPIU_QUERY_OPCODE_READ_FLAG:
+ ioctl_data->buf_size = 1;
+ data_ptr = &flag;
+ break;
+ default:
+ BUG_ON(true);
+ }
+
+ /* copy to user */
+ err = copy_to_user(buffer, ioctl_data,
+ sizeof(struct ufs_ioctl_query_data));
+ if (err)
+ dev_err(hba->dev, "%s: Failed copying back to user.\n",
+ __func__);
+ err = copy_to_user(buffer + sizeof(struct ufs_ioctl_query_data),
+ data_ptr, ioctl_data->buf_size);
+ if (err)
+ dev_err(hba->dev, "%s: err %d copying back to user.\n",
+ __func__, err);
+ goto out_release_mem;
+
+out_einval:
+ dev_err(hba->dev,
+ "%s: illegal ufs query ioctl data, opcode 0x%x, idn 0x%x\n",
+ __func__, ioctl_data->opcode, (unsigned int)ioctl_data->idn);
+ err = -EINVAL;
+out_release_mem:
+ kfree(ioctl_data);
+ kfree(desc);
+out:
+ return err;
+}
+
+/**
+ * ufshcd_ioctl - ufs ioctl callback registered in scsi_host
+ * @dev: scsi device required for per LUN queries
+ * @cmd: command opcode
+ * @buffer: user space buffer for transferring data
+ *
+ * Supported commands:
+ * UFS_IOCTL_QUERY
+ */
+static int ufshcd_ioctl(struct scsi_device *dev, int cmd, void __user *buffer)
+{
+ struct ufs_hba *hba = shost_priv(dev->host);
+ int err = 0;
+
+ BUG_ON(!hba);
+ if (!buffer) {
+ dev_err(hba->dev, "%s: User buffer is NULL!\n", __func__);
+ return -EINVAL;
+ }
+
+ switch (cmd) {
+ case UFS_IOCTL_QUERY:
+ pm_runtime_get_sync(hba->dev);
+ err = ufshcd_query_ioctl(hba, ufshcd_scsi_to_upiu_lun(dev->lun),
+ buffer);
+ pm_runtime_put_sync(hba->dev);
+ break;
+ case UFS_IOCTL_BLKROSET:
+ err = -ENOIOCTLCMD;
+ break;
+ default:
+ err = -EINVAL;
+ dev_err(hba->dev, "%s: Illegal ufs-IOCTL cmd %d\n", __func__,
+ cmd);
+ break;
+ }
+
+ return err;
+}
+
static struct scsi_host_template ufshcd_driver_template = {
.module = THIS_MODULE,
.name = UFSHCD,
@@ -4213,6 +4433,7 @@ static struct scsi_host_template ufshcd_driver_template = {
.eh_abort_handler = ufshcd_abort,
.eh_device_reset_handler = ufshcd_eh_device_reset_handler,
.eh_host_reset_handler = ufshcd_eh_host_reset_handler,
+ .ioctl = ufshcd_ioctl,
.this_id = -1,
.sg_tablesize = SG_ALL,
.cmd_per_lun = UFSHCD_CMD_PER_LUN,
diff --git a/include/uapi/scsi/Kbuild b/include/uapi/scsi/Kbuild
index 75746d5..d404525 100644
--- a/include/uapi/scsi/Kbuild
+++ b/include/uapi/scsi/Kbuild
@@ -1,5 +1,6 @@
# UAPI Header export list
header-y += fc/
+header-y += ufs/
header-y += scsi_bsg_fc.h
header-y += scsi_netlink.h
header-y += scsi_netlink_fc.h
diff --git a/include/uapi/scsi/ufs/Kbuild b/include/uapi/scsi/ufs/Kbuild
new file mode 100644
index 0000000..cc3ef20
--- /dev/null
+++ b/include/uapi/scsi/ufs/Kbuild
@@ -0,0 +1,3 @@
+# UAPI Header export list
+header-y += ioctl.h
+header-y += ufs.h
diff --git a/include/uapi/scsi/ufs/ioctl.h b/include/uapi/scsi/ufs/ioctl.h
new file mode 100644
index 0000000..bc4eed7
--- /dev/null
+++ b/include/uapi/scsi/ufs/ioctl.h
@@ -0,0 +1,57 @@
+#ifndef UAPI_UFS_IOCTL_H_
+#define UAPI_UFS_IOCTL_H_
+
+#include <linux/types.h>
+
+/*
+ * IOCTL opcode for ufs queries has the following opcode after
+ * SCSI_IOCTL_GET_PCI
+ */
+#define UFS_IOCTL_QUERY 0x5388
+
+/**
+ * struct ufs_ioctl_query_data - used to transfer data to and from user via ioctl
+ * @opcode: type of data to query (descriptor/attribute/flag)
+ * @idn: id of the data structure
+ * @buf_size: number of allocated bytes/data size on return
+ * @buffer: data location
+ *
+ * Received: buffer and buf_size (available space for transferred data)
+ * Submitted: opcode, idn, length, buf_size
+ */
+struct ufs_ioctl_query_data {
+ /*
+ * User should select one of the opcode defined in "enum query_opcode".
+ * Please check include/uapi/scsi/ufs/ufs.h for the definition of it.
+ * Note that only UPIU_QUERY_OPCODE_READ_DESC,
+ * UPIU_QUERY_OPCODE_READ_ATTR & UPIU_QUERY_OPCODE_READ_FLAG are
+ * supported as of now. All other query_opcode would be considered
+ * invalid.
+ * As of now only read query operations are supported.
+ */
+ __u32 opcode;
+ /*
+ * User should select one of the idn from "enum flag_idn" or "enum
+ * attr_idn" or "enum desc_idn" based on whether opcode above is
+ * attribute, flag or descriptor.
+ * Please check include/uapi/scsi/ufs/ufs.h for the definition of it.
+ */
+ __u8 idn;
+ /*
+ * User should specify the size of the buffer (buffer[0] below) where
+ * it wants to read the query data (attribute/flag/descriptor).
+ * As we might end up reading less data then what is specified in
+ * buf_size. So we are updating buf_size to what exactly we have read.
+ */
+ __u16 buf_size;
+ /*
+ * placeholder for the start of the data buffer where kernel will copy
+ * the query data (attribute/flag/descriptor) read from the UFS device
+ * Note:
+ * For Read Attribute you will have to allocate 4 bytes
+ * For Read Flag you will have to allocate 1 byte
+ */
+ __u8 buffer[0];
+};
+
+#endif /* UAPI_UFS_IOCTL_H_ */
diff --git a/include/uapi/scsi/ufs/ufs.h b/include/uapi/scsi/ufs/ufs.h
new file mode 100644
index 0000000..894ea45
--- /dev/null
+++ b/include/uapi/scsi/ufs/ufs.h
@@ -0,0 +1,66 @@
+#ifndef UAPI_UFS_H_
+#define UAPI_UFS_H_
+
+/* Flag idn for Query Requests*/
+enum flag_idn {
+ QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+ QUERY_FLAG_IDN_PERMANENT_WPE = 0x02,
+ QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
+ QUERY_FLAG_IDN_BKOPS_EN = 0x04,
+ QUERY_FLAG_IDN_RESERVED1 = 0x05,
+ QUERY_FLAG_IDN_PURGE_ENABLE = 0x06,
+ QUERY_FLAG_IDN_RESERVED2 = 0x07,
+ QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL = 0x08,
+ QUERY_FLAG_IDN_BUSY_RTC = 0x09,
+};
+
+/* Attribute idn for Query requests */
+enum attr_idn {
+ QUERY_ATTR_IDN_BOOT_LU_EN = 0x00,
+ QUERY_ATTR_IDN_RESERVED = 0x01,
+ QUERY_ATTR_IDN_POWER_MODE = 0x02,
+ QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
+ QUERY_ATTR_IDN_OOO_DATA_EN = 0x04,
+ QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
+ QUERY_ATTR_IDN_PURGE_STATUS = 0x06,
+ QUERY_ATTR_IDN_MAX_DATA_IN = 0x07,
+ QUERY_ATTR_IDN_MAX_DATA_OUT = 0x08,
+ QUERY_ATTR_IDN_DYN_CAP_NEEDED = 0x09,
+ QUERY_ATTR_IDN_REF_CLK_FREQ = 0x0A,
+ QUERY_ATTR_IDN_CONF_DESC_LOCK = 0x0B,
+ QUERY_ATTR_IDN_MAX_NUM_OF_RTT = 0x0C,
+ QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
+ QUERY_ATTR_IDN_EE_STATUS = 0x0E,
+ QUERY_ATTR_IDN_SECONDS_PASSED = 0x0F,
+ QUERY_ATTR_IDN_CNTX_CONF = 0x10,
+ QUERY_ATTR_IDN_CORR_PRG_BLK_NUM = 0x11,
+};
+
+/* Descriptor idn for Query requests */
+enum desc_idn {
+ QUERY_DESC_IDN_DEVICE = 0x0,
+ QUERY_DESC_IDN_CONFIGURAION = 0x1,
+ QUERY_DESC_IDN_UNIT = 0x2,
+ QUERY_DESC_IDN_RFU_0 = 0x3,
+ QUERY_DESC_IDN_INTERCONNECT = 0x4,
+ QUERY_DESC_IDN_STRING = 0x5,
+ QUERY_DESC_IDN_RFU_1 = 0x6,
+ QUERY_DESC_IDN_GEOMETRY = 0x7,
+ QUERY_DESC_IDN_POWER = 0x8,
+ QUERY_DESC_IDN_RFU_2 = 0x9,
+ QUERY_DESC_IDN_MAX,
+};
+
+/* UTP QUERY Transaction Specific Fields OpCode */
+enum query_opcode {
+ UPIU_QUERY_OPCODE_NOP = 0x0,
+ UPIU_QUERY_OPCODE_READ_DESC = 0x1,
+ UPIU_QUERY_OPCODE_WRITE_DESC = 0x2,
+ UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
+ UPIU_QUERY_OPCODE_WRITE_ATTR = 0x4,
+ UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
+ UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
+ UPIU_QUERY_OPCODE_CLEAR_FLAG = 0x7,
+ UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
+};
+#endif /* UAPI_UFS_H_ */
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [RFC PATCH 1/3] eeprom: Add a simple EEPROM framework
From: Sascha Hauer @ 2015-02-23 9:15 UTC (permalink / raw)
To: Rob Herring
Cc: Srinivas Kandagatla,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Maxime Ripard, Rob Herring, Pawel Moll, Kumar Gala,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stephen Boyd,
Arnd Bergmann, Mark Brown, Greg Kroah-Hartman
In-Reply-To: <CAL_Jsq+mvpRjYfL_8OseTDCB-6aBhwhNKLBQXXJeVQLDwWm8Nw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Feb 20, 2015 at 04:01:55PM -0600, Rob Herring wrote:
> On Fri, Feb 20, 2015 at 1:25 PM, Srinivas Kandagatla
> <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >
> >
> > On 20/02/15 17:21, Rob Herring wrote:
> >>
> >> On Thu, Feb 19, 2015 at 11:08 AM, Srinivas Kandagatla
> >> <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >>>
> >>> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>>
> >>> Up until now, EEPROM drivers were stored in drivers/misc, where they all
> >>> had to
> >>> duplicate pretty much the same code to register a sysfs file, allow
> >>> in-kernel
> >>> users to access the content of the devices they were driving, etc.
> >>>
> >>> This was also a problem as far as other in-kernel users were involved,
> >>> since
> >>> the solutions used were pretty much different from on driver to another,
> >>> there
> >>> was a rather big abstraction leak.
> >>>
> >>> This introduction of this framework aims at solving this. It also
> >>> introduces DT
> >>> representation for consumer devices to go get the data they require (MAC
> >>> Addresses, SoC/Revision ID, part numbers, and so on) from the EEPROMs.
> >>>
> >>> Having regmap interface to this framework would give much better
> >>> abstraction for eeproms on different buses.
> >>>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>> [srinivas.kandagatla: Moved to regmap based and cleanedup apis]
> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>> ---
> >>> .../devicetree/bindings/eeprom/eeprom.txt | 48 ++++
> >>> drivers/Kconfig | 2 +
> >>> drivers/Makefile | 1 +
> >>> drivers/eeprom/Kconfig | 19 ++
> >>> drivers/eeprom/Makefile | 9 +
> >>> drivers/eeprom/core.c | 290
> >>> +++++++++++++++++++++
> >>> include/linux/eeprom-consumer.h | 73 ++++++
> >>> include/linux/eeprom-provider.h | 51 ++++
> >>
> >>
> >> Who is going to be the maintainer for this?
> >
> >
> > Am happy to be one.
>
> So please add a MAINTAINERS entry.
>
> [...]
>
> >>> += Data consumers =
> >>> +
> >>> +Required properties:
> >>> +
> >>> +eeproms: List of phandle and data cell specifier triplet, one triplet
> >>> + for each data cell the device might be interested in. The
> >>> + triplet consists of the phandle to the eeprom provider, then
> >>> + the offset in byte within that storage device, and the length
> >>> + in byte of the data we care about.
> >>
> >>
> >> The problem with this is it assumes you know who the consumer is and
> >> that it is a DT node. For example, how would you describe a serial
> >> number?
> >
> > Correct me if I miss understood.
> > Is serial number any different?
> > Am hoping that the eeprom consumer would be aware of offset and size of
> > serial number in the eeprom
> >
> > Cant the consumer do:
> >
> > eeprom-consumer {
> > eeproms = <&at24 0 4>;
> > eeprom-names = "device-serial-number";
>
> Yes, but who is "eeprom-consumer"? DT nodes generally describe a h/w
> block, but it this case, the consumer depends on the OS, not the h/w.
> I'm not saying you can't describe where things are, but I don't think
> you should imply who is the consumer and doing so is unnecessarily
> complicated.
>
> Also, the layout of EEPROM is likely very much platform specific. Some
> could have a more complex structure perhaps with key ids and linked
> list structure.
>
> I would do something more simple that is just a list of keys and their
> location like this:
>
> device-serial-number = <start size>;
> key1 = <start size>;
> key2 = <start size>;
Maybe better a node instead of a property. That would allow to use the
standard reg property to describe the position in the eeprom. Also it
would give consumers in dt a possibility to use a phandle to point to a
variable:
serial_number: var@c {
reg = <0xc 0x8>;
name = <?>;
};
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v3 1/4] scsi: ufs: add ioctl interface for query request
From: Dov Levenglick @ 2015-02-23 9:16 UTC (permalink / raw)
To: Gilad Broner
Cc: james.bottomley, linux-kernel, linux-scsi, linux-arm-msm,
santoshsy, linux-scsi-owner, subhashj, ygardi, draviv, Noa Rubens,
Raviv Shvili, Vinayak Holikatti, James E.J. Bottomley,
open list:ABI/API
In-Reply-To: <1424678898-3723-2-git-send-email-gbroner@codeaurora.org>
> From: Dolev Raviv <draviv@codeaurora.org>
>
> This patch exposes the ioctl interface for UFS driver via SCSI device
> ioctl interface. As of now UFS driver would provide the ioctl for query
> interface to connected UFS device.
>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> Signed-off-by: Noa Rubens <noag@codeaurora.org>
> Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
> ---
> drivers/scsi/ufs/ufs.h | 53 +++-------
> drivers/scsi/ufs/ufshcd.c | 225
> +++++++++++++++++++++++++++++++++++++++++-
> include/uapi/scsi/Kbuild | 1 +
> include/uapi/scsi/ufs/Kbuild | 3 +
> include/uapi/scsi/ufs/ioctl.h | 57 +++++++++++
> include/uapi/scsi/ufs/ufs.h | 66 +++++++++++++
> 6 files changed, 361 insertions(+), 44 deletions(-)
> create mode 100644 include/uapi/scsi/ufs/Kbuild
> create mode 100644 include/uapi/scsi/ufs/ioctl.h
> create mode 100644 include/uapi/scsi/ufs/ufs.h
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 42c459a..1f023c4 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -38,6 +38,7 @@
>
> #include <linux/mutex.h>
> #include <linux/types.h>
> +#include <scsi/ufs/ufs.h>
>
> #define MAX_CDB_SIZE 16
> #define GENERAL_UPIU_REQUEST_SIZE 32
> @@ -71,6 +72,16 @@ enum {
> UFS_UPIU_RPMB_WLUN = 0xC4,
> };
>
> +/**
> + * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit
> descriptor
> + * @lun: LU number to check
> + * @return: true if the lun has a matching unit descriptor, false
> otherwise
> + */
> +static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
> +{
> + return (lun == UFS_UPIU_RPMB_WLUN || (lun <
> UFS_UPIU_MAX_GENERAL_LUN));
> +}
> +
> /*
> * UFS Protocol Information Unit related definitions
> */
> @@ -126,35 +137,6 @@ enum {
> UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
> };
>
> -/* Flag idn for Query Requests*/
> -enum flag_idn {
> - QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
> - QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
> - QUERY_FLAG_IDN_BKOPS_EN = 0x04,
> -};
> -
> -/* Attribute idn for Query requests */
> -enum attr_idn {
> - QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
> - QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
> - QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
> - QUERY_ATTR_IDN_EE_STATUS = 0x0E,
> -};
> -
> -/* Descriptor idn for Query requests */
> -enum desc_idn {
> - QUERY_DESC_IDN_DEVICE = 0x0,
> - QUERY_DESC_IDN_CONFIGURAION = 0x1,
> - QUERY_DESC_IDN_UNIT = 0x2,
> - QUERY_DESC_IDN_RFU_0 = 0x3,
> - QUERY_DESC_IDN_INTERCONNECT = 0x4,
> - QUERY_DESC_IDN_STRING = 0x5,
> - QUERY_DESC_IDN_RFU_1 = 0x6,
> - QUERY_DESC_IDN_GEOMETRY = 0x7,
> - QUERY_DESC_IDN_POWER = 0x8,
> - QUERY_DESC_IDN_MAX,
> -};
> -
> enum desc_header_offset {
> QUERY_DESC_LENGTH_OFFSET = 0x00,
> QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
> @@ -247,19 +229,6 @@ enum bkops_status {
> BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
> };
>
> -/* UTP QUERY Transaction Specific Fields OpCode */
> -enum query_opcode {
> - UPIU_QUERY_OPCODE_NOP = 0x0,
> - UPIU_QUERY_OPCODE_READ_DESC = 0x1,
> - UPIU_QUERY_OPCODE_WRITE_DESC = 0x2,
> - UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
> - UPIU_QUERY_OPCODE_WRITE_ATTR = 0x4,
> - UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
> - UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
> - UPIU_QUERY_OPCODE_CLEAR_FLAG = 0x7,
> - UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
> -};
> -
> /* Query response result code */
> enum {
> QUERY_RESULT_SUCCESS = 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 5d60a86..cb357f8 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3,7 +3,7 @@
> *
> * This code is based on drivers/scsi/ufs/ufshcd.c
> * Copyright (C) 2011-2013 Samsung India Software Operations
> - * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
> *
> * Authors:
> * Santosh Yaraganavi <santosh.sy@samsung.com>
> @@ -39,6 +39,7 @@
>
> #include <linux/async.h>
> #include <linux/devfreq.h>
> +#include <scsi/ufs/ioctl.h>
>
> #include "ufshcd.h"
> #include "unipro.h"
> @@ -74,6 +75,9 @@
> /* Interrupt aggregation default timeout, unit: 40us */
> #define INT_AGGR_DEF_TO 0x02
>
> +/* IOCTL opcode for command - ufs set device read only */
> +#define UFS_IOCTL_BLKROSET BLKROSET
> +
> #define ufshcd_toggle_vreg(_dev, _vreg, _on) \
> ({ \
> int _ret; \
> @@ -1882,7 +1886,7 @@ static inline int ufshcd_read_unit_desc_param(struct
> ufs_hba *hba,
> * Unit descriptors are only available for general purpose LUs
> (LUN id
> * from 0 to 7) and RPMB Well known LU.
> */
> - if (lun != UFS_UPIU_RPMB_WLUN && (lun >=
> UFS_UPIU_MAX_GENERAL_LUN))
> + if (!ufs_is_valid_unit_desc_lun(lun))
> return -EOPNOTSUPP;
>
> return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
> @@ -4201,6 +4205,222 @@ static void ufshcd_async_scan(void *data,
> async_cookie_t cookie)
> ufshcd_probe_hba(hba);
> }
>
> +/**
> + * ufshcd_query_ioctl - perform user read queries
> + * @hba: per-adapter instance
> + * @lun: used for lun specific queries
> + * @buffer: user space buffer for reading and submitting query data and
> params
> + * @return: 0 for success negative error code otherwise
> + *
> + * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
> + * It will read the opcode, idn and buf_length parameters, and, put the
> + * response in the buffer field while updating the used size in
> buf_length.
> + */
> +static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user
> *buffer)
> +{
> + struct ufs_ioctl_query_data *ioctl_data;
> + int err = 0;
> + int length = 0;
> + void *data_ptr;
> + bool flag;
> + u32 att;
> + u8 index;
> + u8 *desc = NULL;
> +
> + ioctl_data = kzalloc(sizeof(struct ufs_ioctl_query_data),
> GFP_KERNEL);
> + if (!ioctl_data) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + /* extract params from user buffer */
> + err = copy_from_user(ioctl_data, buffer,
> + sizeof(struct ufs_ioctl_query_data));
> + if (err) {
> + dev_err(hba->dev,
> + "%s: Failed copying buffer from user, err %d\n",
> + __func__, err);
> + goto out_release_mem;
> + }
> +
> + /* verify legal parameters & send query */
> + switch (ioctl_data->opcode) {
> + case UPIU_QUERY_OPCODE_READ_DESC:
> + switch (ioctl_data->idn) {
> + case QUERY_DESC_IDN_DEVICE:
> + case QUERY_DESC_IDN_CONFIGURAION:
> + case QUERY_DESC_IDN_INTERCONNECT:
> + case QUERY_DESC_IDN_GEOMETRY:
> + case QUERY_DESC_IDN_POWER:
> + index = 0;
> + break;
> + case QUERY_DESC_IDN_UNIT:
> + if (!ufs_is_valid_unit_desc_lun(lun)) {
> + dev_err(hba->dev,
> + "%s: No unit descriptor for lun
> 0x%x\n",
> + __func__, lun);
> + err = -EINVAL;
> + goto out_release_mem;
> + }
> + index = lun;
> + break;
> + default:
> + goto out_einval;
> + }
> + length = min_t(int, QUERY_DESC_MAX_SIZE,
> + ioctl_data->buf_size);
> + desc = kzalloc(length, GFP_KERNEL);
> + if (!desc) {
> + dev_err(hba->dev, "%s: Failed allocating %d
> bytes\n",
> + __func__, length);
> + err = -ENOMEM;
> + goto out_release_mem;
> + }
> + err = ufshcd_query_descriptor(hba, ioctl_data->opcode,
> + ioctl_data->idn, index, 0, desc, &length);
> + break;
> + case UPIU_QUERY_OPCODE_READ_ATTR:
> + switch (ioctl_data->idn) {
> + case QUERY_ATTR_IDN_BOOT_LU_EN:
> + case QUERY_ATTR_IDN_POWER_MODE:
> + case QUERY_ATTR_IDN_ACTIVE_ICC_LVL:
> + case QUERY_ATTR_IDN_OOO_DATA_EN:
> + case QUERY_ATTR_IDN_BKOPS_STATUS:
> + case QUERY_ATTR_IDN_PURGE_STATUS:
> + case QUERY_ATTR_IDN_MAX_DATA_IN:
> + case QUERY_ATTR_IDN_MAX_DATA_OUT:
> + case QUERY_ATTR_IDN_REF_CLK_FREQ:
> + case QUERY_ATTR_IDN_CONF_DESC_LOCK:
> + case QUERY_ATTR_IDN_MAX_NUM_OF_RTT:
> + case QUERY_ATTR_IDN_EE_CONTROL:
> + case QUERY_ATTR_IDN_EE_STATUS:
> + case QUERY_ATTR_IDN_SECONDS_PASSED:
> + index = 0;
> + break;
> + case QUERY_ATTR_IDN_DYN_CAP_NEEDED:
> + case QUERY_ATTR_IDN_CORR_PRG_BLK_NUM:
> + index = lun;
> + break;
> + default:
> + goto out_einval;
> + }
> + err = ufshcd_query_attr(hba, ioctl_data->opcode,
> + ioctl_data->idn, index, 0, &att);
> + break;
> + case UPIU_QUERY_OPCODE_READ_FLAG:
> + switch (ioctl_data->idn) {
> + case QUERY_FLAG_IDN_FDEVICEINIT:
> + case QUERY_FLAG_IDN_PERMANENT_WPE:
> + case QUERY_FLAG_IDN_PWR_ON_WPE:
> + case QUERY_FLAG_IDN_BKOPS_EN:
> + case QUERY_FLAG_IDN_PURGE_ENABLE:
> + case QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL:
> + case QUERY_FLAG_IDN_BUSY_RTC:
> + break;
> + default:
> + goto out_einval;
> + }
> + err = ufshcd_query_flag(hba, ioctl_data->opcode,
> + ioctl_data->idn, &flag);
> + break;
> + default:
> + goto out_einval;
> + }
> +
> + if (err) {
> + dev_err(hba->dev, "%s: Query for idn %d failed\n",
> __func__,
> + ioctl_data->idn);
> + goto out_release_mem;
> + }
> +
> + /*
> + * copy response data
> + * As we might end up reading less data then what is specified in
> + * "ioct_data->buf_size". So we are updating "ioct_data->
> + * buf_size" to what exactly we have read.
> + */
> + switch (ioctl_data->opcode) {
> + case UPIU_QUERY_OPCODE_READ_DESC:
> + ioctl_data->buf_size = min_t(int, ioctl_data->buf_size,
> length);
> + data_ptr = desc;
> + break;
> + case UPIU_QUERY_OPCODE_READ_ATTR:
> + ioctl_data->buf_size = sizeof(u32);
> + data_ptr = &att;
> + break;
> + case UPIU_QUERY_OPCODE_READ_FLAG:
> + ioctl_data->buf_size = 1;
> + data_ptr = &flag;
> + break;
> + default:
> + BUG_ON(true);
> + }
> +
> + /* copy to user */
> + err = copy_to_user(buffer, ioctl_data,
> + sizeof(struct ufs_ioctl_query_data));
> + if (err)
> + dev_err(hba->dev, "%s: Failed copying back to user.\n",
> + __func__);
> + err = copy_to_user(buffer + sizeof(struct ufs_ioctl_query_data),
> + data_ptr, ioctl_data->buf_size);
> + if (err)
> + dev_err(hba->dev, "%s: err %d copying back to user.\n",
> + __func__, err);
> + goto out_release_mem;
> +
> +out_einval:
> + dev_err(hba->dev,
> + "%s: illegal ufs query ioctl data, opcode 0x%x, idn
> 0x%x\n",
> + __func__, ioctl_data->opcode, (unsigned
> int)ioctl_data->idn);
> + err = -EINVAL;
> +out_release_mem:
> + kfree(ioctl_data);
> + kfree(desc);
> +out:
> + return err;
> +}
> +
> +/**
> + * ufshcd_ioctl - ufs ioctl callback registered in scsi_host
> + * @dev: scsi device required for per LUN queries
> + * @cmd: command opcode
> + * @buffer: user space buffer for transferring data
> + *
> + * Supported commands:
> + * UFS_IOCTL_QUERY
> + */
> +static int ufshcd_ioctl(struct scsi_device *dev, int cmd, void __user
> *buffer)
> +{
> + struct ufs_hba *hba = shost_priv(dev->host);
> + int err = 0;
> +
> + BUG_ON(!hba);
> + if (!buffer) {
> + dev_err(hba->dev, "%s: User buffer is NULL!\n", __func__);
> + return -EINVAL;
> + }
> +
> + switch (cmd) {
> + case UFS_IOCTL_QUERY:
> + pm_runtime_get_sync(hba->dev);
> + err = ufshcd_query_ioctl(hba,
> ufshcd_scsi_to_upiu_lun(dev->lun),
> + buffer);
> + pm_runtime_put_sync(hba->dev);
> + break;
> + case UFS_IOCTL_BLKROSET:
> + err = -ENOIOCTLCMD;
> + break;
> + default:
> + err = -EINVAL;
> + dev_err(hba->dev, "%s: Illegal ufs-IOCTL cmd %d\n",
> __func__,
> + cmd);
> + break;
> + }
> +
> + return err;
> +}
> +
> static struct scsi_host_template ufshcd_driver_template = {
> .module = THIS_MODULE,
> .name = UFSHCD,
> @@ -4213,6 +4433,7 @@ static struct scsi_host_template
> ufshcd_driver_template = {
> .eh_abort_handler = ufshcd_abort,
> .eh_device_reset_handler = ufshcd_eh_device_reset_handler,
> .eh_host_reset_handler = ufshcd_eh_host_reset_handler,
> + .ioctl = ufshcd_ioctl,
> .this_id = -1,
> .sg_tablesize = SG_ALL,
> .cmd_per_lun = UFSHCD_CMD_PER_LUN,
> diff --git a/include/uapi/scsi/Kbuild b/include/uapi/scsi/Kbuild
> index 75746d5..d404525 100644
> --- a/include/uapi/scsi/Kbuild
> +++ b/include/uapi/scsi/Kbuild
> @@ -1,5 +1,6 @@
> # UAPI Header export list
> header-y += fc/
> +header-y += ufs/
> header-y += scsi_bsg_fc.h
> header-y += scsi_netlink.h
> header-y += scsi_netlink_fc.h
> diff --git a/include/uapi/scsi/ufs/Kbuild b/include/uapi/scsi/ufs/Kbuild
> new file mode 100644
> index 0000000..cc3ef20
> --- /dev/null
> +++ b/include/uapi/scsi/ufs/Kbuild
> @@ -0,0 +1,3 @@
> +# UAPI Header export list
> +header-y += ioctl.h
> +header-y += ufs.h
> diff --git a/include/uapi/scsi/ufs/ioctl.h b/include/uapi/scsi/ufs/ioctl.h
> new file mode 100644
> index 0000000..bc4eed7
> --- /dev/null
> +++ b/include/uapi/scsi/ufs/ioctl.h
> @@ -0,0 +1,57 @@
> +#ifndef UAPI_UFS_IOCTL_H_
> +#define UAPI_UFS_IOCTL_H_
> +
> +#include <linux/types.h>
> +
> +/*
> + * IOCTL opcode for ufs queries has the following opcode after
> + * SCSI_IOCTL_GET_PCI
> + */
> +#define UFS_IOCTL_QUERY 0x5388
> +
> +/**
> + * struct ufs_ioctl_query_data - used to transfer data to and from user
> via ioctl
> + * @opcode: type of data to query (descriptor/attribute/flag)
> + * @idn: id of the data structure
> + * @buf_size: number of allocated bytes/data size on return
> + * @buffer: data location
> + *
> + * Received: buffer and buf_size (available space for transferred data)
> + * Submitted: opcode, idn, length, buf_size
> + */
> +struct ufs_ioctl_query_data {
> + /*
> + * User should select one of the opcode defined in "enum
> query_opcode".
> + * Please check include/uapi/scsi/ufs/ufs.h for the definition of
> it.
> + * Note that only UPIU_QUERY_OPCODE_READ_DESC,
> + * UPIU_QUERY_OPCODE_READ_ATTR & UPIU_QUERY_OPCODE_READ_FLAG are
> + * supported as of now. All other query_opcode would be considered
> + * invalid.
> + * As of now only read query operations are supported.
> + */
> + __u32 opcode;
> + /*
> + * User should select one of the idn from "enum flag_idn" or "enum
> + * attr_idn" or "enum desc_idn" based on whether opcode above is
> + * attribute, flag or descriptor.
> + * Please check include/uapi/scsi/ufs/ufs.h for the definition of
> it.
> + */
> + __u8 idn;
> + /*
> + * User should specify the size of the buffer (buffer[0] below)
> where
> + * it wants to read the query data (attribute/flag/descriptor).
> + * As we might end up reading less data then what is specified in
> + * buf_size. So we are updating buf_size to what exactly we have
> read.
> + */
> + __u16 buf_size;
> + /*
> + * placeholder for the start of the data buffer where kernel will
> copy
> + * the query data (attribute/flag/descriptor) read from the UFS
> device
> + * Note:
> + * For Read Attribute you will have to allocate 4 bytes
> + * For Read Flag you will have to allocate 1 byte
> + */
> + __u8 buffer[0];
> +};
> +
> +#endif /* UAPI_UFS_IOCTL_H_ */
> diff --git a/include/uapi/scsi/ufs/ufs.h b/include/uapi/scsi/ufs/ufs.h
> new file mode 100644
> index 0000000..894ea45
> --- /dev/null
> +++ b/include/uapi/scsi/ufs/ufs.h
> @@ -0,0 +1,66 @@
> +#ifndef UAPI_UFS_H_
> +#define UAPI_UFS_H_
> +
> +/* Flag idn for Query Requests*/
> +enum flag_idn {
> + QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
> + QUERY_FLAG_IDN_PERMANENT_WPE = 0x02,
> + QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
> + QUERY_FLAG_IDN_BKOPS_EN = 0x04,
> + QUERY_FLAG_IDN_RESERVED1 = 0x05,
> + QUERY_FLAG_IDN_PURGE_ENABLE = 0x06,
> + QUERY_FLAG_IDN_RESERVED2 = 0x07,
> + QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL = 0x08,
> + QUERY_FLAG_IDN_BUSY_RTC = 0x09,
> +};
> +
> +/* Attribute idn for Query requests */
> +enum attr_idn {
> + QUERY_ATTR_IDN_BOOT_LU_EN = 0x00,
> + QUERY_ATTR_IDN_RESERVED = 0x01,
> + QUERY_ATTR_IDN_POWER_MODE = 0x02,
> + QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
> + QUERY_ATTR_IDN_OOO_DATA_EN = 0x04,
> + QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
> + QUERY_ATTR_IDN_PURGE_STATUS = 0x06,
> + QUERY_ATTR_IDN_MAX_DATA_IN = 0x07,
> + QUERY_ATTR_IDN_MAX_DATA_OUT = 0x08,
> + QUERY_ATTR_IDN_DYN_CAP_NEEDED = 0x09,
> + QUERY_ATTR_IDN_REF_CLK_FREQ = 0x0A,
> + QUERY_ATTR_IDN_CONF_DESC_LOCK = 0x0B,
> + QUERY_ATTR_IDN_MAX_NUM_OF_RTT = 0x0C,
> + QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
> + QUERY_ATTR_IDN_EE_STATUS = 0x0E,
> + QUERY_ATTR_IDN_SECONDS_PASSED = 0x0F,
> + QUERY_ATTR_IDN_CNTX_CONF = 0x10,
> + QUERY_ATTR_IDN_CORR_PRG_BLK_NUM = 0x11,
> +};
> +
> +/* Descriptor idn for Query requests */
> +enum desc_idn {
> + QUERY_DESC_IDN_DEVICE = 0x0,
> + QUERY_DESC_IDN_CONFIGURAION = 0x1,
> + QUERY_DESC_IDN_UNIT = 0x2,
> + QUERY_DESC_IDN_RFU_0 = 0x3,
> + QUERY_DESC_IDN_INTERCONNECT = 0x4,
> + QUERY_DESC_IDN_STRING = 0x5,
> + QUERY_DESC_IDN_RFU_1 = 0x6,
> + QUERY_DESC_IDN_GEOMETRY = 0x7,
> + QUERY_DESC_IDN_POWER = 0x8,
> + QUERY_DESC_IDN_RFU_2 = 0x9,
> + QUERY_DESC_IDN_MAX,
> +};
> +
> +/* UTP QUERY Transaction Specific Fields OpCode */
> +enum query_opcode {
> + UPIU_QUERY_OPCODE_NOP = 0x0,
> + UPIU_QUERY_OPCODE_READ_DESC = 0x1,
> + UPIU_QUERY_OPCODE_WRITE_DESC = 0x2,
> + UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
> + UPIU_QUERY_OPCODE_WRITE_ATTR = 0x4,
> + UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
> + UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
> + UPIU_QUERY_OPCODE_CLEAR_FLAG = 0x7,
> + UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
> +};
> +#endif /* UAPI_UFS_H_ */
> --
> Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Reviewed-by: Dov Levenglick <dovl@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 05/45] drm.h: include stdlib.h in userspace
From: Emil Velikov @ 2015-02-23 10:26 UTC (permalink / raw)
To: Mikko Rapeli, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1424127948-22484-6-git-send-email-mikko.rapeli-X3B1VOXEql0@public.gmane.org>
On 16/02/15 23:05, Mikko Rapeli wrote:
> Fixes <drm/drm.h> compilation error:
>
> drm/drm.h:132:2: error: unknown type name ‘size_t’
>
Hi Mikko,
Can you let us know how you're getting these (series-wise) errors ? I've
been meaning to sync the uapi/drm and libdrm headers and would be nice
to have an extra step to test things.
Thanks
Emil
^ permalink raw reply
* Re: [PATCH v2 02/18] ARM: ARMv7M: Enlarge vector table to 256 entries
From: Maxime Coquelin @ 2015-02-23 10:33 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andreas Färber, Geert Uytterhoeven, Rob Herring,
Philipp Zabel, Jonathan Corbet, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon
In-Reply-To: <20150220194756.GS19388@pengutronix.de>
2015-02-20 20:47 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> On Fri, Feb 20, 2015 at 07:01:01PM +0100, Maxime Coquelin wrote:
>> From Cortex-M reference manuals, the nvic supports up to 240 interrupts.
>> So the number of entries in vectors table is up to 256.
>>
>> This patch adds a new config flag to specify the number of external interrupts.
>> Some ifdeferies are added in order to respect the natural alignment without
>> wasting too much space on smaller systems.
>>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>> arch/arm/kernel/entry-v7m.S | 13 +++++++++----
>> arch/arm/mm/Kconfig | 15 +++++++++++++++
>> 2 files changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
>> index 8944f49..68cde36 100644
>> --- a/arch/arm/kernel/entry-v7m.S
>> +++ b/arch/arm/kernel/entry-v7m.S
>> @@ -117,9 +117,14 @@ ENTRY(__switch_to)
>> ENDPROC(__switch_to)
>>
>> .data
>> - .align 8
>> +#if CONFIG_CPUV7M_NUM_IRQ <= 112
> I would have called this CONFIG_CPU_V7M_NUM_IRQ to match the already
> existing CPU_V7M symbol.
That's better indeed.
It will be changed in v3.
>
>> + .align 9
>> +#else
>> + .align 10
>> +#endif
>> +
>
> Other than that:
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> Who do you target to apply this series?
I guess it should go through Russell's Patch Tracking System?
Thanks,
Maxime
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 05/45] drm.h: include stdlib.h in userspace
From: Mikko Rapeli @ 2015-02-23 10:35 UTC (permalink / raw)
To: Emil Velikov
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54EB0072.8020909-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Feb 23, 2015 at 10:26:58AM +0000, Emil Velikov wrote:
> On 16/02/15 23:05, Mikko Rapeli wrote:
> > Fixes <drm/drm.h> compilation error:
> >
> > drm/drm.h:132:2: error: unknown type name ‘size_t’
> >
> Hi Mikko,
>
> Can you let us know how you're getting these (series-wise) errors ? I've
> been meaning to sync the uapi/drm and libdrm headers and would be nice
> to have an extra step to test things.
This should have everything needed to reproduce these compile errors,
though some of the errors hide behind other errors and fixes:
https://lkml.org/lkml/2015/2/16/525
-Mikko
^ permalink raw reply
* Re: Documenting MS_LAZYTIME
From: Austin S Hemmelgarn @ 2015-02-23 12:20 UTC (permalink / raw)
To: Theodore Ts'o, Eric Sandeen
Cc: Michael Kerrisk, Ext4 Developers List,
Linux btrfs Developers List, XFS Developers,
linux-man-u79uwXL29TY76Z2rM5mHXA, Linux-Fsdevel, Linux API
In-Reply-To: <20150221025636.GB7922-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
On 2015-02-20 21:56, Theodore Ts'o wrote:
> On Fri, Feb 20, 2015 at 09:49:34AM -0600, Eric Sandeen wrote:
>>
>>> This mount option significantly reduces writes to the
>>> inode table for workloads that perform frequent random
>>> writes to preallocated files.
>>
>> This seems like an overly specific description of a single workload out
>> of many which may benefit, but what do others think? "inode table" is also
>> fairly extN-specific.
>
> How about somethign like "This mount significantly reduces writes
> needed to update the inode's timestamps, especially mtime and actime.
> Examples of workloads where this could be a large win include frequent
> random writes to preallocated files, as well as cases where the
> MS_STRICTATIME mount option is enabled."?
>
> (The advantage of MS_STRICTATIME | MS_LAZYTIME is that stat system
> calls will return the correctly updated atime, but those atime updates
> won't get flushed to disk unless the inode needs to be updated for
> file system / data consistency reasons, or when the inode is pushed
> out of memory, or when the file system is unmounted.)
>
If you want to list some specific software, it should help with anything
that uses sqlite (which notably includes firefox and chrome), as well as
most RDMS software and systemd-journald.
^ permalink raw reply
* Re: [PATCH 1/3] media: Fix ALSA and DVB representation at media controller API
From: Mauro Carvalho Chehab @ 2015-02-23 13:55 UTC (permalink / raw)
To: Devin Heitmueller
Cc: Hans Verkuil, Linux Media Mailing List, Mauro Carvalho Chehab,
Hans Verkuil, Sakari Ailus, Antti Palosaari, Ricardo Ribalda,
Marek Szyprowski, Ramakrishnan Muthukrishnan, Laurent Pinchart,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAGoCfiwi0nj_9sYNzEFOp5BvedFe+HphJ2bVtx_bnBw3d-Bsyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Em Mon, 26 Jan 2015 09:41:41 -0500
Devin Heitmueller <dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg@public.gmane.org> escreveu:
> > It is actually trivial to get the device nodes once you have the
> > major/minor. The media-ctl library does that for you. See:
>
> No objection then.
>
> On a related note, you would be very well served to consider testing
> your dvb changes with a device that has more than one DVB tuner (such
> as the hvr-2200/2250). That will help you shake out any edge cases
> related to ensuring that the different DVB nodes appear in different
> groups.
Hi Devin,
I did some tests (and fixes) for WinTV Nova-TD, with has two adapters.
I saw two alternatives for it:
1) to create a media controller device for each adapter;
2) to create just one media controller.
I actually implemented (1), as, in the case of this device, AFAIKT, the
two devices are indepentent, e. g. it is not possible to, for example,
share the same tuner with two demods:
$ ls -la /dev/media?
crw-rw----. 1 root video 249, 0 Fev 23 10:02 /dev/media0
crw-rw----. 1 root video 249, 1 Fev 23 10:02 /dev/media1
The adapter 0 corresponds to /dev/media0, and the adapter 1
to /dev/media1:
$ media-ctl --print-dot -d /dev/media0
digraph board {
rankdir=TB
n00000001 [label="dvb-demux\n/dev/dvb/adapter0/demux0", shape=box, style=filled, fillcolor=yellow]
n00000001 -> n00000002
n00000002 [label="dvb-dvr\n/dev/dvb/adapter0/dvr0", shape=box, style=filled, fillcolor=yellow]
n00000003 [label="dvb-net\n/dev/dvb/adapter0/net0", shape=box, style=filled, fillcolor=yellow]
n00000004 [label="DiBcom 7000PC\n/dev/dvb/adapter0/frontend0", shape=box, style=filled, fillcolor=yellow]
n00000004 -> n00000001
}
$ media-ctl --print-dot -d /dev/media1
digraph board {
rankdir=TB
n00000001 [label="dvb-demux\n/dev/dvb/adapter1/demux0", shape=box, style=filled, fillcolor=yellow]
n00000001 -> n00000002
n00000002 [label="dvb-dvr\n/dev/dvb/adapter1/dvr0", shape=box, style=filled, fillcolor=yellow]
n00000003 [label="dvb-net\n/dev/dvb/adapter1/net0", shape=box, style=filled, fillcolor=yellow]
n00000004 [label="DiBcom 7000PC\n/dev/dvb/adapter1/frontend0", shape=box, style=filled, fillcolor=yellow]
n00000004 -> n00000001
}
On a more complex hardware where some components may be rewired
on a different way, however, using just one media controller
would be a better approach.
Comments?
Mauro
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: pcm512x: Allow independently overclocking PLL, DAC and DSP
From: Mark Brown @ 2015-02-23 14:31 UTC (permalink / raw)
To: Peter Rosin
Cc: Peter Rosin, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Lars-Peter Clausen,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <88616b158f38499fa5a466f31d25aa80-RtsSNOqGLlYRm0/ZmH0nkw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]
On Sun, Feb 22, 2015 at 12:24:12AM +0000, Peter Rosin wrote:
> ...but I'm not sure everybody agrees that overclocking games should
> be allowed by any and all users?
I don't see why not, ASoC controls are already way beyond end user a lot
of the time.
> If you still want me to convert to ALSA controls, what control do you
> suggest? SOC_SINGLE_EXT? Or should I use an enumeration, because
> mixers tend to present volume controls as a percentage of max, which
> will be confusing: You are now at "volume" 75% (of max 40), when the
> value is 30. Eeek. But enumerations from 0% to 40% sounds tedious.
I'd just use a single value, it's a UI problem with the mixer and if the
control isn't called "Volume" that shouldn cause the UI to not present
it as a volume. He said hopefully.
Or possibly a binary control I guess which would have the nice side
effect of hiding from most non-specialist UIs.
> And how would you suggest that I name the controls?
> "Max Overclock DAC", "Max Overclock DSP" and "Max Overclock PLL"?
Those sound reasonable.
> BTW, the only troubles I've had with overclocking "too much" is that it
> has stopped working. I have not managed to fry any chip. But that is no
> guarantee, of course.
It's vanishingly unlikely that you'll cause physical damage with this
sort of thing, you're much more likely to hit performance and filter
problems than anything else.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 14:58 UTC (permalink / raw)
To: Serge Hallyn
Cc: Andy Lutomirski, Aaron Jones, Ted Ts'o,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502051554500.4876-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org>
Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
weeks without comment after an intensive discussion about the RFCs.
Since there were no objections: Is there any chance to get this into -next
somehow?
^ permalink raw reply
* Re: [RFC PATCH 1/3] eeprom: Add a simple EEPROM framework
From: Mark Brown @ 2015-02-23 15:04 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Rob Herring, Pawel Moll, Kumar Gala,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd, Arnd Bergmann,
Greg Kroah-Hartman
In-Reply-To: <1424365708-26681-1-git-send-email-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2967 bytes --]
On Thu, Feb 19, 2015 at 05:08:28PM +0000, Srinivas Kandagatla wrote:
> .../devicetree/bindings/eeprom/eeprom.txt | 48 ++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/eeprom/Kconfig | 19 ++
> drivers/eeprom/Makefile | 9 +
> drivers/eeprom/core.c | 290 +++++++++++++++++++++
> include/linux/eeprom-consumer.h | 73 ++++++
> include/linux/eeprom-provider.h | 51 ++++
This seems to have a bunch of different things in it - there's some
binding, there's some framework code, there's some user code for the
framework... splitting it up more would probably help with review.
I'd at least make sure the framework is split from the DT code, that
will cut down on the bulk and help make sure the framework isn't too DT
tied.
> + if (read)
> + rc = regmap_bulk_read(eeprom->regmap, offset,
> + buf, count/eeprom->stride);
> + else
> + rc = regmap_bulk_write(eeprom->regmap, offset,
> + buf, count/eeprom->stride);
> +
> + if (IS_ERR_VALUE(rc))
> + return 0;
> +
> + return count;
> +}
> +static ssize_t bin_attr_eeprom_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *attr,
> + char *buf, loff_t offset, size_t count)
> +{
> + return bin_attr_eeprom_read_write(kobj, buf, offset, count, true);
> +}
I'm not sure the factoring out is actually helping the clarity here TBH.
> +int eeprom_unregister(struct eeprom_device *eeprom)
> +{
> + device_del(&eeprom->edev);
> +
> + mutex_lock(&eeprom_list_mutex);
> + list_del(&eeprom->list);
> + mutex_unlock(&eeprom_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(eeprom_unregister);
Here we return having dropped the lock without doing anything else with
the eeprom, meaning the caller could delete it.
> + mutex_lock(&eeprom_list_mutex);
> +
> + list_for_each_entry(e, &eeprom_list, list) {
> + if (args.np == e->edev.of_node) {
> + eeprom = e;
> + break;
> + }
> + }
> + mutex_unlock(&eeprom_list_mutex);
Here we iterate the list, find the relevant eeprom and drop the lock
while still holding a reference to the eeprom we just found. This means
that a removal could race with us and free the eeprom underneath us.
I'm also not seeing anything stopping or even noticing a device being
removed with a cell active on it.
> +/**
> + * eeprom_cell_get(): Get eeprom cell of device form a given index.
> + *
> + * @dev: Device that will be interacted with
> + * @index: Index of the eeprom cell.
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer
> + * to a struct eeprom_cell. The eeprom_cell will be freed by the
> + * eeprom_cell_put().
> + */
> +struct eeprom_cell *eeprom_cell_get(struct device *dev, int index);
Normally the kerneldoc goes with the function definition, not the
prototype.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 1/3] eeprom: Add a simple EEPROM framework
From: Srinivas Kandagatla @ 2015-02-23 15:38 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Rob Herring, Pawel Moll, Kumar Gala,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd, Arnd Bergmann,
Greg Kroah-Hartman
In-Reply-To: <20150223150415.GF6236-bheZrs9scGb3/WHNxyQH9YN0K6Il/+VY@public.gmane.org>
Thankyou for the comments.
On 23/02/15 15:04, Mark Brown wrote:
> On Thu, Feb 19, 2015 at 05:08:28PM +0000, Srinivas Kandagatla wrote:
>
>> .../devicetree/bindings/eeprom/eeprom.txt | 48 ++++
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 1 +
>> drivers/eeprom/Kconfig | 19 ++
>> drivers/eeprom/Makefile | 9 +
>> drivers/eeprom/core.c | 290 +++++++++++++++++++++
>> include/linux/eeprom-consumer.h | 73 ++++++
>> include/linux/eeprom-provider.h | 51 ++++
>
> This seems to have a bunch of different things in it - there's some
> binding, there's some framework code, there's some user code for the
> framework... splitting it up more would probably help with review.
> I'd at least make sure the framework is split from the DT code, that
> will cut down on the bulk and help make sure the framework isn't too DT
> tied.
Yes I agree, will make sure that I split it into different patches in
next version.
>
>> + if (read)
>> + rc = regmap_bulk_read(eeprom->regmap, offset,
>> + buf, count/eeprom->stride);
>> + else
>> + rc = regmap_bulk_write(eeprom->regmap, offset,
>> + buf, count/eeprom->stride);
>> +
>> + if (IS_ERR_VALUE(rc))
>> + return 0;
>> +
>> + return count;
>> +}
>
>> +static ssize_t bin_attr_eeprom_read(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *attr,
>> + char *buf, loff_t offset, size_t count)
>> +{
>> + return bin_attr_eeprom_read_write(kobj, buf, offset, count, true);
>> +}
>
> I'm not sure the factoring out is actually helping the clarity here TBH.
>
ok.
>> +int eeprom_unregister(struct eeprom_device *eeprom)
>> +{
>> + device_del(&eeprom->edev);
>> +
>> + mutex_lock(&eeprom_list_mutex);
>> + list_del(&eeprom->list);
>> + mutex_unlock(&eeprom_list_mutex);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(eeprom_unregister);
>
> Here we return having dropped the lock without doing anything else with
> the eeprom, meaning the caller could delete it.
>
>> + mutex_lock(&eeprom_list_mutex);
>> +
>> + list_for_each_entry(e, &eeprom_list, list) {
>> + if (args.np == e->edev.of_node) {
>> + eeprom = e;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&eeprom_list_mutex);
>
> Here we iterate the list, find the relevant eeprom and drop the lock
> while still holding a reference to the eeprom we just found. This means
> that a removal could race with us and free the eeprom underneath us.
> I'm also not seeing anything stopping or even noticing a device being
> removed with a cell active on it.
>
As suggested by Stephen Boyd reference counting on eeprom should ensure
safe removal of eeprom.
>> +/**
>> + * eeprom_cell_get(): Get eeprom cell of device form a given index.
>> + *
>> + * @dev: Device that will be interacted with
>> + * @index: Index of the eeprom cell.
>> + *
>> + * The return value will be an ERR_PTR() on error or a valid pointer
>> + * to a struct eeprom_cell. The eeprom_cell will be freed by the
>> + * eeprom_cell_put().
>> + */
>> +struct eeprom_cell *eeprom_cell_get(struct device *dev, int index);
>
> Normally the kerneldoc goes with the function definition, not the
> prototype.
Thats true, will fix it in next version.
>
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Andy Lutomirski @ 2015-02-23 15:44 UTC (permalink / raw)
To: Christoph Lameter
Cc: Serge Hallyn, Aaron Jones, Ted Ts'o, LSM List, Andrew Morton,
Andrew G. Morgan, Mimi Zohar, Austin S Hemmelgarn, Markku Savela,
Jarkko Sakkinen, linux-kernel@vger.kernel.org, Linux API,
Michael Kerrisk, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502230856400.21238@gentwo.org>
On Mon, Feb 23, 2015 at 6:58 AM, Christoph Lameter <cl@linux.com> wrote:
> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> weeks without comment after an intensive discussion about the RFCs.
>
> Since there were no objections: Is there any chance to get this into -next
> somehow?
>
At the very least, I think it needs to define and implement what
happens when a cap is added to ambient and then dropped from
permitted. We also may need LSM_UNSAFE_something to clear the ambient
set to avoid a major security issue.
I'd like to discuss (in the hallway if nothing else) at LSF/MM with
whatever other interested people will be there.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 15:53 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Aaron Jones, Ted Ts'o, LSM List, Andrew Morton,
Andrew G. Morgan, Mimi Zohar, Austin S Hemmelgarn, Markku Savela,
Jarkko Sakkinen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Michael Kerrisk, Jonathan Corbet
In-Reply-To: <CALCETrWJCcBBGGp21C4cdtiU79K-P3t+6rFJUWcXcLR1jqrrFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 23 Feb 2015, Andy Lutomirski wrote:
> At the very least, I think it needs to define and implement what
> happens when a cap is added to ambient and then dropped from
> permitted. We also may need LSM_UNSAFE_something to clear the ambient
> set to avoid a major security issue.
The ambient cap needs to stay otherwise we will have issues again if
another binary/script is forked. IMHO the only way to switch off an
ambient capability should be another prctl action. The intend is after all
to have the cap available for all inherited processes.
Frankly, I'd rather have the ambient caps separate. We could do a stronger
separation by checking for permitted or ambient in capable().
> I'd like to discuss (in the hallway if nothing else) at LSF/MM with
> whatever other interested people will be there.
Ok. I will be at the MM meeting in Boston. 8th and 9th of March.
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Andy Lutomirski @ 2015-02-23 15:59 UTC (permalink / raw)
To: Christoph Lameter
Cc: Serge Hallyn, Aaron Jones, Ted Ts'o, LSM List, Andrew Morton,
Andrew G. Morgan, Mimi Zohar, Austin S Hemmelgarn, Markku Savela,
Jarkko Sakkinen, linux-kernel@vger.kernel.org, Linux API,
Michael Kerrisk, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502230950440.21238@gentwo.org>
On Mon, Feb 23, 2015 at 7:53 AM, Christoph Lameter <cl@linux.com> wrote:
> On Mon, 23 Feb 2015, Andy Lutomirski wrote:
>
>> At the very least, I think it needs to define and implement what
>> happens when a cap is added to ambient and then dropped from
>> permitted. We also may need LSM_UNSAFE_something to clear the ambient
>> set to avoid a major security issue.
>
> The ambient cap needs to stay otherwise we will have issues again if
> another binary/script is forked. IMHO the only way to switch off an
> ambient capability should be another prctl action. The intend is after all
> to have the cap available for all inherited processes.
No, sadly.
If you set ambient caps and then run a setuid program (without
no_new_privs), then the ambient set *must* be cleared by the kernel
because that's what the setuid program expects. Yes, the whole
concept of setuid is daft, but it exists and we have to keep it
working safely, or at least as safely as it ever worked.
--Andy
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Serge Hallyn @ 2015-02-23 16:16 UTC (permalink / raw)
To: Christoph Lameter
Cc: Serge Hallyn, Andy Lutomirski, Aaron Jones, Ted Ts'o,
linux-security-module, akpm, Andrew G. Morgan, Mimi Zohar,
Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen, linux-kernel,
linux-api, Michael Kerrisk, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502230856400.21238@gentwo.org>
Quoting Christoph Lameter (cl@linux.com):
> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> weeks without comment after an intensive discussion about the RFCs.
>
> Since there were no objections: Is there any chance to get this into -next
> somehow?
Andrew Morgan and Andy Lutomirski appear to have a similar concern
but competing ideas on how to address them. We need them to agree
on an approach.
The core concern for amorgan is that an unprivileged user not be
able to cause a privileged program to run in a way that it fails to
drop privilege before running unprivileged-user-provided code.
Andy Lutomirski's concern is simply that code which is currently
doing the right thing to drop privilege not be run in a way that
it thinks it is dropping privilege, but in fact is not.
(Please correct me where I've mis-spoken or misunderstood)
Since your desire is precisely for a mode where dropping privilege
works as usual, but exec then re-gains some or all of that privilege,
we need to either agree on a way to enter that mode that ordinary
use caes can't be tricked into using, or find a way for legacy
users to be tpiped off as to what's going on (without having to be
re-written)
-serge
^ permalink raw reply
* Re: Documenting MS_LAZYTIME
From: Eric Sandeen @ 2015-02-23 16:24 UTC (permalink / raw)
To: Austin S Hemmelgarn, Theodore Ts'o
Cc: Michael Kerrisk, Ext4 Developers List,
Linux btrfs Developers List, XFS Developers,
linux-man-u79uwXL29TY76Z2rM5mHXA, Linux-Fsdevel, Linux API
In-Reply-To: <54EB1B19.8050808-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 2/23/15 6:20 AM, Austin S Hemmelgarn wrote:
> On 2015-02-20 21:56, Theodore Ts'o wrote:
>> On Fri, Feb 20, 2015 at 09:49:34AM -0600, Eric Sandeen wrote:
>>>
>>>> This mount option significantly reduces writes to the
>>>> inode table for workloads that perform frequent random
>>>> writes to preallocated files.
>>>
>>> This seems like an overly specific description of a single workload out
>>> of many which may benefit, but what do others think? "inode table" is also
>>> fairly extN-specific.
>>
>> How about somethign like "This mount significantly reduces writes
>> needed to update the inode's timestamps, especially mtime and actime.
>> Examples of workloads where this could be a large win include frequent
>> random writes to preallocated files, as well as cases where the
>> MS_STRICTATIME mount option is enabled."?
>>
>> (The advantage of MS_STRICTATIME | MS_LAZYTIME is that stat system
>> calls will return the correctly updated atime, but those atime updates
>> won't get flushed to disk unless the inode needs to be updated for
>> file system / data consistency reasons, or when the inode is pushed
>> out of memory, or when the file system is unmounted.)
>>
> If you want to list some specific software, it should help with
> anything that uses sqlite (which notably includes firefox and
> chrome), as well as most RDMS software and systemd-journald.
I'm really uneasy with starting to list specific workloads and applications
here. It's going to get dated quickly, and will lead to endless cargo-cult
tuning.
I'd strongly prefer to just describe what it does (reduces the number of
certain metadata writes to disk) and leave it at that....
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Andy Lutomirski @ 2015-02-23 16:33 UTC (permalink / raw)
To: Serge Hallyn
Cc: Christoph Lameter, Serge Hallyn, Aaron Jones, Ted Ts'o,
LSM List, Andrew Morton, Andrew G. Morgan, Mimi Zohar,
Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel@vger.kernel.org, Linux API, Michael Kerrisk,
Jonathan Corbet
In-Reply-To: <20150223161625.GD25477@ubuntumail>
On Mon, Feb 23, 2015 at 8:16 AM, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
> Quoting Christoph Lameter (cl@linux.com):
>> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
>> weeks without comment after an intensive discussion about the RFCs.
>>
>> Since there were no objections: Is there any chance to get this into -next
>> somehow?
>
> Andrew Morgan and Andy Lutomirski appear to have a similar concern
> but competing ideas on how to address them. We need them to agree
> on an approach.
>
> The core concern for amorgan is that an unprivileged user not be
> able to cause a privileged program to run in a way that it fails to
> drop privilege before running unprivileged-user-provided code.
>
> Andy Lutomirski's concern is simply that code which is currently
> doing the right thing to drop privilege not be run in a way that
> it thinks it is dropping privilege, but in fact is not.
>
I share both concerns.
> (Please correct me where I've mis-spoken or misunderstood)
>
> Since your desire is precisely for a mode where dropping privilege
> works as usual, but exec then re-gains some or all of that privilege,
> we need to either agree on a way to enter that mode that ordinary
> use caes can't be tricked into using, or find a way for legacy
> users to be tpiped off as to what's going on (without having to be
> re-written)
Is there really a need to drop privilege and then regain it or is it
sufficient to keep the privilege permitted (and perhaps ambient, too)
and just to have execve not drop it for you? I assume the latter.
--Andy
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 16:41 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Aaron Jones, Ted Ts'o, LSM List, Andrew Morton,
Andrew G. Morgan, Mimi Zohar, Austin S Hemmelgarn, Markku Savela,
Jarkko Sakkinen, linux-kernel@vger.kernel.org, Linux API,
Michael Kerrisk, Jonathan Corbet
In-Reply-To: <CALCETrUR7+44ie2fiy0BzC3ktLQ2dcArSL-O3AHnp8kFF92fjg@mail.gmail.com>
On Mon, 23 Feb 2015, Andy Lutomirski wrote:
> If you set ambient caps and then run a setuid program (without
> no_new_privs), then the ambient set *must* be cleared by the kernel
> because that's what the setuid program expects. Yes, the whole
Why would a setuid program expect that? I'd say we expect the ambient set
to remain in effect. What would break if the ambient set would stay
active?
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 16:44 UTC (permalink / raw)
To: Serge Hallyn
Cc: Serge Hallyn, Andy Lutomirski, Aaron Jones, Ted Ts'o,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Jonathan Corbet
In-Reply-To: <20150223161625.GD25477@ubuntumail>
On Mon, 23 Feb 2015, Serge Hallyn wrote:
> The core concern for amorgan is that an unprivileged user not be
> able to cause a privileged program to run in a way that it fails to
> drop privilege before running unprivileged-user-provided code.
I do not see a problem with dropping privilege since the ambient set
is supposed to be preserved across a drop of priviledge.
> Since your desire is precisely for a mode where dropping privilege
> works as usual, but exec then re-gains some or all of that privilege,
I would say that the ambient set stays active even if the setuid binary
drops to regular perms.
> we need to either agree on a way to enter that mode that ordinary
> use caes can't be tricked into using, or find a way for legacy
> users to be tpiped off as to what's going on (without having to be
> re-written)
Well if the ambient set is completely separate then the existing
semantics are preserved while the ambient set stays active as intended.
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Serge E. Hallyn @ 2015-02-23 16:45 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Christoph Lameter, Serge Hallyn, Aaron Jones,
Ted Ts'o, LSM List, Andrew Morton, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Michael Kerrisk, Jonathan Corbet
In-Reply-To: <CALCETrW78805ayUL=ZYBdFwVdDvJZus2JL0VVmEBE8=L1Nm5Sw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Feb 23, 2015 at 08:33:58AM -0800, Andy Lutomirski wrote:
> On Mon, Feb 23, 2015 at 8:16 AM, Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote:
> > Quoting Christoph Lameter (cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org):
> >> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> >> weeks without comment after an intensive discussion about the RFCs.
> >>
> >> Since there were no objections: Is there any chance to get this into -next
> >> somehow?
> >
> > Andrew Morgan and Andy Lutomirski appear to have a similar concern
> > but competing ideas on how to address them. We need them to agree
> > on an approach.
> >
> > The core concern for amorgan is that an unprivileged user not be
> > able to cause a privileged program to run in a way that it fails to
> > drop privilege before running unprivileged-user-provided code.
> >
> > Andy Lutomirski's concern is simply that code which is currently
> > doing the right thing to drop privilege not be run in a way that
> > it thinks it is dropping privilege, but in fact is not.
> >
>
> I share both concerns.
>
> > (Please correct me where I've mis-spoken or misunderstood)
> >
> > Since your desire is precisely for a mode where dropping privilege
> > works as usual, but exec then re-gains some or all of that privilege,
> > we need to either agree on a way to enter that mode that ordinary
> > use caes can't be tricked into using, or find a way for legacy
> > users to be tpiped off as to what's going on (without having to be
> > re-written)
>
> Is there really a need to drop privilege and then regain it or is it
> sufficient to keep the privilege permitted (and perhaps ambient, too)
> and just to have execve not drop it for you? I assume the latter.
Well right, any perceived security benefit of the temporary drop would
seem to be easily debunked (just run shell for exec /bin/sh to get
around it)
So this is more of a desire, I suspect, for regular programs which
drop privilege to still be usable in this environment.
I think this may be a decent place for a compromise. Attempts to
drop privilege when ambient caps are set return EPERM.
-serge
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Serge E. Hallyn @ 2015-02-23 16:46 UTC (permalink / raw)
To: Christoph Lameter
Cc: Serge Hallyn, Serge Hallyn, Andy Lutomirski, Aaron Jones,
Ted Ts'o, linux-security-module, akpm, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel, linux-api, Michael Kerrisk, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502231041580.21784@gentwo.org>
On Mon, Feb 23, 2015 at 10:44:32AM -0600, Christoph Lameter wrote:
> On Mon, 23 Feb 2015, Serge Hallyn wrote:
>
> > The core concern for amorgan is that an unprivileged user not be
> > able to cause a privileged program to run in a way that it fails to
> > drop privilege before running unprivileged-user-provided code.
>
> I do not see a problem with dropping privilege since the ambient set
> is supposed to be preserved across a drop of priviledge.
Because you're tricking the program into thinking it has dropped
the privilege, when in fact it has not.
> > Since your desire is precisely for a mode where dropping privilege
> > works as usual, but exec then re-gains some or all of that privilege,
>
> I would say that the ambient set stays active even if the setuid binary
> drops to regular perms.
>
> > we need to either agree on a way to enter that mode that ordinary
> > use caes can't be tricked into using, or find a way for legacy
> > users to be tpiped off as to what's going on (without having to be
> > re-written)
>
> Well if the ambient set is completely separate then the existing
> semantics are preserved while the ambient set stays active as intended.
>
> --
> 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/
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 16:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Serge Hallyn, Serge Hallyn, Aaron Jones, Ted Ts'o, LSM List,
Andrew Morton, Andrew G. Morgan, Mimi Zohar, Austin S Hemmelgarn,
Markku Savela, Jarkko Sakkinen, linux-kernel@vger.kernel.org,
Linux API, Michael Kerrisk, Jonathan Corbet
In-Reply-To: <CALCETrW78805ayUL=ZYBdFwVdDvJZus2JL0VVmEBE8=L1Nm5Sw@mail.gmail.com>
On Mon, 23 Feb 2015, Andy Lutomirski wrote:
> Is there really a need to drop privilege and then regain it or is it
> sufficient to keep the privilege permitted (and perhaps ambient, too)
> and just to have execve not drop it for you? I assume the latter.
I would think just keep the ambient set active as long as there is no
prctl switching the cap off in the child processes. Do not let it be
affected by the usual drop privs stuff.
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Christoph Lameter @ 2015-02-23 16:50 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Serge Hallyn, Serge Hallyn, Andy Lutomirski, Aaron Jones,
Ted Ts'o, linux-security-module, akpm, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel, linux-api, Michael Kerrisk, Jonathan Corbet
In-Reply-To: <20150223164623.GB32181@mail.hallyn.com>
On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
> > I do not see a problem with dropping privilege since the ambient set
> > is supposed to be preserved across a drop of priviledge.
>
> Because you're tricking the program into thinking it has dropped
> the privilege, when in fact it has not.
So the cap was dropped from the cap perm set but it is still active
in the ambient set?
^ permalink raw reply
* Re: [PATCH] capabilities: Ambient capability set V1
From: Serge Hallyn @ 2015-02-23 18:15 UTC (permalink / raw)
To: Christoph Lameter
Cc: Serge E. Hallyn, Serge Hallyn, Andy Lutomirski, Aaron Jones,
Ted Ts'o, linux-security-module, akpm, Andrew G. Morgan,
Mimi Zohar, Austin S Hemmelgarn, Markku Savela, Jarkko Sakkinen,
linux-kernel, linux-api, Michael Kerrisk, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1502231049110.21926@gentwo.org>
Quoting Christoph Lameter (cl@linux.com):
> On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
>
> > > I do not see a problem with dropping privilege since the ambient set
> > > is supposed to be preserved across a drop of priviledge.
> >
> > Because you're tricking the program into thinking it has dropped
> > the privilege, when in fact it has not.
>
> So the cap was dropped from the cap perm set but it is still active
> in the ambient set?
Right, and the legacy program doesn't know to check the new set.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox